diff --git a/api/v1alpha1/common_types.go b/api/v1alpha1/common_types.go index ac6bed11..73e18cb9 100644 --- a/api/v1alpha1/common_types.go +++ b/api/v1alpha1/common_types.go @@ -354,9 +354,11 @@ type CellName string // +kubebuilder:validation:MaxLength=512 type InitdbArgs string -// PostgresConfigRef references a ConfigMap containing extra postgresql.conf lines -// appended to pgctld's auto-tuned defaults via POSTGRES_INITDB_EXTRA_CONF. -// The referenced ConfigMap must exist in the same namespace as the MultigresCluster. +// PostgresConfigRef references a ConfigMap whose postgresql.conf lines are +// merged into the operator-rendered config. The referenced ConfigMap must exist +// in the same namespace as the MultigresCluster. This type is deprecated in +// favor of the inline spec.postgresConfig map; it remains supported for backward +// compatibility and will be removed in a future version. type PostgresConfigRef struct { // Name is the name of the ConfigMap. // +kubebuilder:validation:MinLength=1 diff --git a/api/v1alpha1/multigrescluster_types.go b/api/v1alpha1/multigrescluster_types.go index 1c9ed660..6ad7390f 100644 --- a/api/v1alpha1/multigrescluster_types.go +++ b/api/v1alpha1/multigrescluster_types.go @@ -394,13 +394,21 @@ type ShardOverrides struct { // +optional InitdbArgs InitdbArgs `json:"initdbArgs,omitempty"` - // PostgresConfigRef references a ConfigMap containing extra postgresql.conf - // lines appended to pgctld's auto-tuned defaults. The ConfigMap must exist in - // the same namespace. When set, the operator mounts it and sets - // POSTGRES_INITDB_EXTRA_CONF on pgctld. + // PostgresConfigRef references a ConfigMap whose postgresql.conf lines are + // merged into the operator-rendered config for this shard's pools. This field + // is deprecated in favor of the inline PostgresConfig map; it remains + // supported for backward compatibility and will be removed in a future + // version. // +optional PostgresConfigRef *PostgresConfigRef `json:"postgresConfigRef,omitempty"` + // PostgresConfig is a map of PostgreSQL parameter (GUC) names to string + // values, merged into the operator-rendered postgresql.conf. It takes + // precedence over PostgresConfigRef and the operator's built-in defaults. + // +optional + // +kubebuilder:validation:MaxProperties=200 + PostgresConfig map[string]string `json:"postgresConfig,omitempty"` + // Pools overrides. Keyed by pool name. // +optional // +kubebuilder:validation:MaxProperties=8 @@ -422,13 +430,21 @@ type ShardInlineSpec struct { // +optional InitdbArgs InitdbArgs `json:"initdbArgs,omitempty"` - // PostgresConfigRef references a ConfigMap containing extra postgresql.conf - // lines appended to pgctld's auto-tuned defaults. The ConfigMap must exist in - // the same namespace. When set, the operator mounts it and sets - // POSTGRES_INITDB_EXTRA_CONF on pgctld. + // PostgresConfigRef references a ConfigMap whose postgresql.conf lines are + // merged into the operator-rendered config for this shard's pools. This field + // is deprecated in favor of the inline PostgresConfig map; it remains + // supported for backward compatibility and will be removed in a future + // version. // +optional PostgresConfigRef *PostgresConfigRef `json:"postgresConfigRef,omitempty"` + // PostgresConfig is a map of PostgreSQL parameter (GUC) names to string + // values, merged into the operator-rendered postgresql.conf. It takes + // precedence over PostgresConfigRef and the operator's built-in defaults. + // +optional + // +kubebuilder:validation:MaxProperties=200 + PostgresConfig map[string]string `json:"postgresConfig,omitempty"` + // Pools configuration. Keyed by pool name. // +optional // +kubebuilder:validation:MaxProperties=8 diff --git a/api/v1alpha1/shard_types.go b/api/v1alpha1/shard_types.go index 8bcd1648..e05af9fb 100644 --- a/api/v1alpha1/shard_types.go +++ b/api/v1alpha1/shard_types.go @@ -144,13 +144,22 @@ type ShardSpec struct { // +optional InitdbArgs InitdbArgs `json:"initdbArgs,omitempty"` - // PostgresConfigRef references a ConfigMap containing extra postgresql.conf - // lines appended to pgctld's auto-tuned defaults. The operator mounts it and - // sets POSTGRES_INITDB_EXTRA_CONF on pgctld; PostgreSQL's last-write-wins - // rule lets you override specific params without replacing the whole config. + // PostgresConfigRef references a ConfigMap whose postgresql.conf lines are + // merged into the operator-rendered config for this shard's pools. This field + // is deprecated in favor of the inline PostgresConfig map; it remains + // supported for backward compatibility and will be removed in a future + // version. // +optional PostgresConfigRef *PostgresConfigRef `json:"postgresConfigRef,omitempty"` + // PostgresConfig is the resolved map of PostgreSQL parameter (GUC) names to + // string values for this shard. The operator renders it over its defaults and + // any PostgresConfigRef content (PostgresConfig wins on key conflicts) into + // the postgresql.conf mounted into pgctld. + // +optional + // +kubebuilder:validation:MaxProperties=200 + PostgresConfig map[string]string `json:"postgresConfig,omitempty"` + // Pools is the map of fully resolved data pool configurations. // +kubebuilder:validation:MaxProperties=8 // +kubebuilder:validation:XValidation:rule="self.all(key, size(key) < 63)",message="pool names must be < 63 chars" @@ -240,12 +249,43 @@ type ShardImages struct { // CR Controller Status Specs // ============================================================================ +// PostgresConfigStatus reports the rollout state of the operator-rendered +// postgresql.conf for a shard, so a caller can tell whether a config change has +// finished rolling out or failed without inspecting pods. +// +// InProgress is content-based: it compares the config effective on the pods +// against the desired render, so it reflects any config change — a spec edit, a +// PostgresConfigRef ConfigMap edit, or a new operator baseline on upgrade — none +// of which are captured by generation alone. A caller checks +// InProgress == false && Error == "" for "settled", and uses the shard's +// top-level status.observedGeneration as the freshness watermark for a spec +// change. +type PostgresConfigStatus struct { + // InProgress is true while the desired rendered config has not yet landed on + // every pool pod (a config rollout is under way). + // +optional + InProgress bool `json:"inProgress,omitempty"` + + // LastAppliedAt is when the rendered config last settled onto every pool pod. + // +optional + LastAppliedAt *metav1.Time `json:"lastAppliedAt,omitempty"` + + // Error is non-empty when the config could not be rendered or read (for + // example, a missing PostgresConfigRef ConfigMap). + // +optional + Error string `json:"error,omitempty"` +} + // ShardStatus defines the observed state of Shard. type ShardStatus struct { // ObservedGeneration is the most recent generation observed. // +optional ObservedGeneration int64 `json:"observedGeneration,omitempty"` + // PostgresConfig reports the rollout state of the rendered postgresql.conf. + // +optional + PostgresConfig *PostgresConfigStatus `json:"postgresConfig,omitempty"` + // Conditions represent the latest available observations. // +optional // +listType=map diff --git a/api/v1alpha1/shardtemplate_types.go b/api/v1alpha1/shardtemplate_types.go index ca036de1..b59c1d96 100644 --- a/api/v1alpha1/shardtemplate_types.go +++ b/api/v1alpha1/shardtemplate_types.go @@ -37,13 +37,21 @@ type ShardTemplateSpec struct { // +optional InitdbArgs InitdbArgs `json:"initdbArgs,omitempty"` - // PostgresConfigRef references a ConfigMap containing extra postgresql.conf - // lines appended to pgctld's auto-tuned defaults. The ConfigMap must exist in - // the same namespace. When set, the operator mounts it and sets - // POSTGRES_INITDB_EXTRA_CONF on pgctld. + // PostgresConfigRef references a ConfigMap whose postgresql.conf lines are + // merged into the operator-rendered config for this shard's pools. This field + // is deprecated in favor of the inline PostgresConfig map; it remains + // supported for backward compatibility and will be removed in a future + // version. // +optional PostgresConfigRef *PostgresConfigRef `json:"postgresConfigRef,omitempty"` + // PostgresConfig is a map of PostgreSQL parameter (GUC) names to string + // values, merged into the operator-rendered postgresql.conf. It takes + // precedence over PostgresConfigRef and the operator's built-in defaults. + // +optional + // +kubebuilder:validation:MaxProperties=200 + PostgresConfig map[string]string `json:"postgresConfig,omitempty"` + // +optional // +kubebuilder:validation:MaxProperties=8 // +kubebuilder:validation:XValidation:rule="self.all(key, size(key) < 63)",message="pool names must be < 63 chars" diff --git a/api/v1alpha1/tablegroup_types.go b/api/v1alpha1/tablegroup_types.go index 7379bdf9..a0288008 100644 --- a/api/v1alpha1/tablegroup_types.go +++ b/api/v1alpha1/tablegroup_types.go @@ -122,12 +122,22 @@ type ShardResolvedSpec struct { // +optional InitdbArgs InitdbArgs `json:"initdbArgs,omitempty"` - // PostgresConfigRef references a ConfigMap containing extra postgresql.conf - // lines appended to pgctld's auto-tuned defaults. When set, the operator - // mounts it and sets POSTGRES_INITDB_EXTRA_CONF on pgctld. + // PostgresConfigRef references a ConfigMap whose postgresql.conf lines are + // merged into the operator-rendered config for this shard's pools. This field + // is deprecated in favor of the inline PostgresConfig map; it remains + // supported for backward compatibility and will be removed in a future + // version. // +optional PostgresConfigRef *PostgresConfigRef `json:"postgresConfigRef,omitempty"` + // PostgresConfig is the resolved map of PostgreSQL parameter (GUC) names to + // string values, merged per-key through the shard template override chain. + // The operator renders it over its defaults and any PostgresConfigRef content + // (PostgresConfig wins) into the postgresql.conf mounted into pgctld. + // +optional + // +kubebuilder:validation:MaxProperties=200 + PostgresConfig map[string]string `json:"postgresConfig,omitempty"` + // Pools is the map of fully resolved data pool configurations. // +kubebuilder:validation:MaxProperties=8 // +kubebuilder:validation:XValidation:rule="self.all(key, size(key) < 63)",message="pool names must be < 63 chars" diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index df281f29..845c3978 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -1209,6 +1209,25 @@ func (in *PostgresConfigRef) DeepCopy() *PostgresConfigRef { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *PostgresConfigStatus) DeepCopyInto(out *PostgresConfigStatus) { + *out = *in + if in.LastAppliedAt != nil { + in, out := &in.LastAppliedAt, &out.LastAppliedAt + *out = (*in).DeepCopy() + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new PostgresConfigStatus. +func (in *PostgresConfigStatus) DeepCopy() *PostgresConfigStatus { + if in == nil { + return nil + } + out := new(PostgresConfigStatus) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *PostgresPasswordSecretRef) DeepCopyInto(out *PostgresPasswordSecretRef) { *out = *in @@ -1370,6 +1389,13 @@ func (in *ShardInlineSpec) DeepCopyInto(out *ShardInlineSpec) { *out = new(PostgresConfigRef) **out = **in } + if in.PostgresConfig != nil { + in, out := &in.PostgresConfig, &out.PostgresConfig + *out = make(map[string]string, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } if in.Pools != nil { in, out := &in.Pools, &out.Pools *out = make(map[PoolName]PoolSpec, len(*in)) @@ -1439,6 +1465,13 @@ func (in *ShardOverrides) DeepCopyInto(out *ShardOverrides) { *out = new(PostgresConfigRef) **out = **in } + if in.PostgresConfig != nil { + in, out := &in.PostgresConfig, &out.PostgresConfig + *out = make(map[string]string, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } if in.Pools != nil { in, out := &in.Pools, &out.Pools *out = make(map[PoolName]PoolSpec, len(*in)) @@ -1467,6 +1500,13 @@ func (in *ShardResolvedSpec) DeepCopyInto(out *ShardResolvedSpec) { *out = new(PostgresConfigRef) **out = **in } + if in.PostgresConfig != nil { + in, out := &in.PostgresConfig, &out.PostgresConfig + *out = make(map[string]string, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } if in.Pools != nil { in, out := &in.Pools, &out.Pools *out = make(map[PoolName]PoolSpec, len(*in)) @@ -1507,6 +1547,13 @@ func (in *ShardSpec) DeepCopyInto(out *ShardSpec) { *out = new(PostgresConfigRef) **out = **in } + if in.PostgresConfig != nil { + in, out := &in.PostgresConfig, &out.PostgresConfig + *out = make(map[string]string, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } if in.Pools != nil { in, out := &in.Pools, &out.Pools *out = make(map[PoolName]PoolSpec, len(*in)) @@ -1579,6 +1626,11 @@ func (in *ShardSpec) DeepCopy() *ShardSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ShardStatus) DeepCopyInto(out *ShardStatus) { *out = *in + if in.PostgresConfig != nil { + in, out := &in.PostgresConfig, &out.PostgresConfig + *out = new(PostgresConfigStatus) + (*in).DeepCopyInto(*out) + } if in.Conditions != nil { in, out := &in.Conditions, &out.Conditions *out = make([]metav1.Condition, len(*in)) @@ -1685,6 +1737,13 @@ func (in *ShardTemplateSpec) DeepCopyInto(out *ShardTemplateSpec) { *out = new(PostgresConfigRef) **out = **in } + if in.PostgresConfig != nil { + in, out := &in.PostgresConfig, &out.PostgresConfig + *out = make(map[string]string, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } if in.Pools != nil { in, out := &in.Pools, &out.Pools *out = make(map[PoolName]PoolSpec, len(*in)) diff --git a/config/crd/bases/multigres.com_multigresclusters.yaml b/config/crd/bases/multigres.com_multigresclusters.yaml index 6d03bbf8..d2ffaa7e 100644 --- a/config/crd/bases/multigres.com_multigresclusters.yaml +++ b/config/crd/bases/multigres.com_multigresclusters.yaml @@ -5499,12 +5499,22 @@ spec: - message: Pools cannot be removed or renamed in this version (Append-Only) rule: oldSelf.all(k, k in self) + postgresConfig: + additionalProperties: + type: string + description: |- + PostgresConfig is a map of PostgreSQL parameter (GUC) names to string + values, merged into the operator-rendered postgresql.conf. It takes + precedence over PostgresConfigRef and the operator's built-in defaults. + maxProperties: 200 + type: object postgresConfigRef: description: |- - PostgresConfigRef references a ConfigMap containing extra postgresql.conf - lines appended to pgctld's auto-tuned defaults. The ConfigMap must exist in - the same namespace. When set, the operator mounts it and sets - POSTGRES_INITDB_EXTRA_CONF on pgctld. + PostgresConfigRef references a ConfigMap whose postgresql.conf lines are + merged into the operator-rendered config for this shard's pools. This field + is deprecated in favor of the inline PostgresConfig map; it remains + supported for backward compatibility and will be removed in a future + version. properties: key: description: Key is the key within the ConfigMap's @@ -7976,12 +7986,22 @@ spec: - message: Pools cannot be removed or renamed in this version (Append-Only) rule: oldSelf.all(k, k in self) + postgresConfig: + additionalProperties: + type: string + description: |- + PostgresConfig is a map of PostgreSQL parameter (GUC) names to string + values, merged into the operator-rendered postgresql.conf. It takes + precedence over PostgresConfigRef and the operator's built-in defaults. + maxProperties: 200 + type: object postgresConfigRef: description: |- - PostgresConfigRef references a ConfigMap containing extra postgresql.conf - lines appended to pgctld's auto-tuned defaults. The ConfigMap must exist in - the same namespace. When set, the operator mounts it and sets - POSTGRES_INITDB_EXTRA_CONF on pgctld. + PostgresConfigRef references a ConfigMap whose postgresql.conf lines are + merged into the operator-rendered config for this shard's pools. This field + is deprecated in favor of the inline PostgresConfig map; it remains + supported for backward compatibility and will be removed in a future + version. properties: key: description: Key is the key within the ConfigMap's diff --git a/config/crd/bases/multigres.com_shards.yaml b/config/crd/bases/multigres.com_shards.yaml index 76abcb40..84288837 100644 --- a/config/crd/bases/multigres.com_shards.yaml +++ b/config/crd/bases/multigres.com_shards.yaml @@ -2740,12 +2740,23 @@ spec: x-kubernetes-validations: - message: pool names must be < 63 chars rule: self.all(key, size(key) < 63) + postgresConfig: + additionalProperties: + type: string + description: |- + PostgresConfig is the resolved map of PostgreSQL parameter (GUC) names to + string values for this shard. The operator renders it over its defaults and + any PostgresConfigRef content (PostgresConfig wins on key conflicts) into + the postgresql.conf mounted into pgctld. + maxProperties: 200 + type: object postgresConfigRef: description: |- - PostgresConfigRef references a ConfigMap containing extra postgresql.conf - lines appended to pgctld's auto-tuned defaults. The operator mounts it and - sets POSTGRES_INITDB_EXTRA_CONF on pgctld; PostgreSQL's last-write-wins - rule lets you override specific params without replacing the whole config. + PostgresConfigRef references a ConfigMap whose postgresql.conf lines are + merged into the operator-rendered config for this shard's pools. This field + is deprecated in favor of the inline PostgresConfig map; it remains + supported for backward compatibility and will be removed in a future + version. properties: key: description: Key is the key within the ConfigMap's data that contains @@ -2978,6 +2989,26 @@ spec: poolsReady: description: PoolsReady indicates if all data pools are ready. type: boolean + postgresConfig: + description: PostgresConfig reports the rollout state of the rendered + postgresql.conf. + properties: + error: + description: |- + Error is non-empty when the config could not be rendered or read (for + example, a missing PostgresConfigRef ConfigMap). + type: string + inProgress: + description: |- + InProgress is true while the desired rendered config has not yet landed on + every pool pod (a config rollout is under way). + type: boolean + lastAppliedAt: + description: LastAppliedAt is when the rendered config last settled + onto every pool pod. + format: date-time + type: string + type: object readyReplicas: description: ReadyReplicas is the total number of ready pods across all pools in this shard. diff --git a/config/crd/bases/multigres.com_shardtemplates.yaml b/config/crd/bases/multigres.com_shardtemplates.yaml index dc68c5bf..f2fcdb99 100644 --- a/config/crd/bases/multigres.com_shardtemplates.yaml +++ b/config/crd/bases/multigres.com_shardtemplates.yaml @@ -2340,12 +2340,22 @@ spec: rule: self.all(key, size(key) < 63) - message: Pools cannot be removed or renamed in this version (Append-Only) rule: oldSelf.all(k, k in self) + postgresConfig: + additionalProperties: + type: string + description: |- + PostgresConfig is a map of PostgreSQL parameter (GUC) names to string + values, merged into the operator-rendered postgresql.conf. It takes + precedence over PostgresConfigRef and the operator's built-in defaults. + maxProperties: 200 + type: object postgresConfigRef: description: |- - PostgresConfigRef references a ConfigMap containing extra postgresql.conf - lines appended to pgctld's auto-tuned defaults. The ConfigMap must exist in - the same namespace. When set, the operator mounts it and sets - POSTGRES_INITDB_EXTRA_CONF on pgctld. + PostgresConfigRef references a ConfigMap whose postgresql.conf lines are + merged into the operator-rendered config for this shard's pools. This field + is deprecated in favor of the inline PostgresConfig map; it remains + supported for backward compatibility and will be removed in a future + version. properties: key: description: Key is the key within the ConfigMap's data that contains diff --git a/config/crd/bases/multigres.com_tablegroups.yaml b/config/crd/bases/multigres.com_tablegroups.yaml index b2807232..d4dd2858 100644 --- a/config/crd/bases/multigres.com_tablegroups.yaml +++ b/config/crd/bases/multigres.com_tablegroups.yaml @@ -2949,11 +2949,23 @@ spec: x-kubernetes-validations: - message: pool names must be < 63 chars rule: self.all(key, size(key) < 63) + postgresConfig: + additionalProperties: + type: string + description: |- + PostgresConfig is the resolved map of PostgreSQL parameter (GUC) names to + string values, merged per-key through the shard template override chain. + The operator renders it over its defaults and any PostgresConfigRef content + (PostgresConfig wins) into the postgresql.conf mounted into pgctld. + maxProperties: 200 + type: object postgresConfigRef: description: |- - PostgresConfigRef references a ConfigMap containing extra postgresql.conf - lines appended to pgctld's auto-tuned defaults. When set, the operator - mounts it and sets POSTGRES_INITDB_EXTRA_CONF on pgctld. + PostgresConfigRef references a ConfigMap whose postgresql.conf lines are + merged into the operator-rendered config for this shard's pools. This field + is deprecated in favor of the inline PostgresConfig map; it remains + supported for backward compatibility and will be removed in a future + version. properties: key: description: Key is the key within the ConfigMap's data diff --git a/docs/postgresql-configuration.md b/docs/postgresql-configuration.md index 3f87c618..c9a80233 100644 --- a/docs/postgresql-configuration.md +++ b/docs/postgresql-configuration.md @@ -1,25 +1,14 @@ # PostgreSQL Configuration -The operator supports custom PostgreSQL runtime configuration via the `postgresConfigRef` field. This lets you provide a ConfigMap containing extra postgresql.conf lines that are appended to pgctld's auto-tuned defaults — PostgreSQL's last-write-wins rule then lets you override specific params without replacing the whole config. +The operator renders each shard's `postgresql.conf` from a built-in baseline plus resource-derived sizing, then layers your overrides on top. You provide overrides with the inline `spec.postgresConfig` map — a set of PostgreSQL parameter (GUC) names to values. + +> **Deprecation:** the older `postgresConfigRef` field (a reference to a ConfigMap of `postgresql.conf` lines) is deprecated in favor of `spec.postgresConfig`. It is still honored for backward compatibility and will be removed in a future version. See [Legacy: postgresConfigRef](#legacy-postgresconfigref). ## Configuration -Create a ConfigMap with your postgresql.conf overrides, then reference it from the shard spec: +Set the parameters you want to override inline on the shard: ```yaml -# User creates their own ConfigMap with postgresql.conf overrides -apiVersion: v1 -kind: ConfigMap -metadata: - name: my-postgres-config -data: - custom.conf: | - shared_buffers = '8GB' - max_connections = 200 - work_mem = '256MB' - ---- -# CRD references it apiVersion: multigres.com/v1alpha1 kind: MultigresCluster metadata: @@ -34,9 +23,9 @@ spec: shards: - name: "0-inf" spec: - postgresConfigRef: - name: my-postgres-config - key: custom.conf + postgresConfig: + max_connections: "200" + work_mem: "16MB" pools: main-rw: type: readWrite @@ -52,9 +41,8 @@ kind: ShardTemplate metadata: name: production spec: - postgresConfigRef: - name: production-pg-config - key: postgresql.conf + postgresConfig: + max_connections: "200" pools: main-rw: type: readWrite @@ -63,38 +51,79 @@ spec: size: "100Gi" ``` +Values are strings in PostgreSQL's own representation (the operator quotes them when rendering, so `"200"` and `"16MB"` are both fine). The field is available at the `ShardTemplate`, `overrides`, and inline `spec` levels and **merges per key** through that chain, with the inline `spec` winning. + ## How It Works -When `postgresConfigRef` is set: +For every shard, the operator renders `postgresql.conf` from these layers, each overriding the one before it (PostgreSQL applies later assignments last-write-wins): + +1. the operator's built-in baseline — a complete small-instance `postgresql.conf` (SSL, logging, locales, `wal_level`, and the tunable defaults), so the operator owns the whole file, not just the knobs you override, +2. resource-derived sizing computed from the shard's CPU/memory and storage (e.g. `shared_buffers`, `effective_cache_size`, WAL sizing) — see [Resource-derived sizing](#resource-derived-sizing), +3. the deprecated `postgresConfigRef` content, if set, +4. the inline `spec.postgresConfig` map — highest precedence. + +The result is written to an operator-owned ConfigMap (`-postgres-config`) mounted into every pool pod; pgctld reads it via the `POSTGRES_INITDB_EXTRA_CONF` env var. Because rendering is always on, every shard has this ConfigMap. pgctld reads the file only when PostgreSQL **starts**, so a config change takes effect by rolling the pods (see [Rolling updates](#rolling-updates)). + +## Resource-derived sizing + +Layer 2 above: the operator sizes the memory-, CPU-, and disk-sensitive parameters from the shard's +resources, so `shared_buffers` and friends scale with the pod instead of sitting at the small-instance +baseline. You don't set these directly — but you can override any of them with `postgresConfig`, which +is higher precedence. + +**Inputs.** Sizing reads each pool's `resources` and `storage.size` and reduces them to one per-shard +basis (config is shard-level — see [Why shard-level?](#why-shard-level)): + +- **memory** and **CPU** — the **maximum** across the shard's pools, taking each pool's **limit** and + falling back to its **request** when no limit is set. Taking the max keeps the replication-sensitive + settings valid on every pod. +- **disk** — the **minimum** `storage.size` across the pools, so WAL budgeting never overfills the + smallest volume. -1. The operator mounts the referenced ConfigMap into every pool pod as a read-only volume -2. The specified key is projected to the `postgresql.conf` filename inside the mount -3. pgctld reads its path from the `POSTGRES_INITDB_EXTRA_CONF` env var and appends the content to its auto-tuned `postgresql.conf` -4. PostgreSQL applies the merged config using last-write-wins, so your overrides win for any param you set +An input that is unset leaves its parameters at the baseline. -When `postgresConfigRef` is not set, pgctld uses only its auto-tuned values based on available resources. No env var is set and no extra volume is mounted. +**What gets sized:** -## Override Chain +| Parameter | From | Formula | +| :------------------------------------------------------------------------ | :----- | :---------------------------------------------------------------------------- | +| `shared_buffers` | memory | `mem / 4` | +| `effective_cache_size` | memory | `mem × 3/4` | +| `maintenance_work_mem` | memory | `min(mem / 16, 2GB)` | +| `wal_buffers` | memory | `clamp(shared_buffers × 3%, 32kB, 16MB)` | +| `work_mem` | memory | `(mem − shared_buffers) / (max_connections × 3) / parallel_workers`, min 64kB | +| `max_worker_processes`, `max_parallel_workers` | CPU | `= cores` | +| `max_parallel_workers_per_gather` | CPU | `= cores / 2` | +| `max_parallel_maintenance_workers` | CPU | `= min(cores / 2, 4)` | +| `min_wal_size`, `max_wal_size`, `wal_keep_size`, `max_slot_wal_keep_size` | disk | scaled down from the volume size | -`postgresConfigRef` uses **last non-nil wins** through the shard template override chain: +Notes: -1. **ShardTemplate** -- base reference -2. **ShardConfig.overrides** -- replaces the reference if set -3. **ShardConfig.spec** (inline) -- replaces the reference if set +- **Parallel-worker settings are tuned only at ≥ 4 CPU cores.** Below that the baseline is kept, so + small pods aren't starved of worker slots. +- **`max_connections` is not resource-derived.** It stays at the baseline so it remains above the + connection pooler's capacity. Raise it explicitly with `postgresConfig` if you need more (and size + the pooler to match). +- These are pgtune-style heuristics; override any of them with `postgresConfig` when your workload + needs something different. -Unlike a key-value map, the ConfigMap reference is an atomic replacement. If you need different parameters for different shards, create separate ConfigMaps. +## Override precedence -### Example +`postgresConfig` **merges per key** through the shard template override chain, so you can set a +baseline in a `ShardTemplate` and override individual parameters lower down: + +1. **ShardTemplate** — base map +2. **ShardConfig.overrides** — merged on top, per key +3. **ShardConfig.spec** (inline) — merged on top, per key; wins on conflicts ```yaml -# ShardTemplate "production" sets baseline +# ShardTemplate "production" sets a baseline spec: - postgresConfigRef: - name: production-pg-config - key: postgresql.conf + postgresConfig: + max_connections: "200" + work_mem: "16MB" --- -# Shard overrides to point at a different ConfigMap +# A shard overrides just one parameter; the rest are inherited spec: databases: - name: postgres @@ -104,58 +133,113 @@ spec: - name: "0-inf" shardTemplate: production overrides: - postgresConfigRef: - name: high-memory-pg-config - key: postgresql.conf + postgresConfig: + work_mem: "64MB" ``` -## Why Shard-Level? +## Why shard-level? -PostgreSQL configuration is defined at the shard level because all pods in a shard replicate from the same primary. A primary and its replicas should have compatible settings -- different `shared_buffers` or `max_connections` across replicas in the same shard creates unpredictable failover behavior. +Configuration is shard-level because all pods in a shard replicate from the same primary. A primary +and its replicas must have compatible settings — hot standby requires several parameters (e.g. +`max_connections`, `max_worker_processes`) on a replica to be at least the primary's, so a uniform +per-shard config keeps failover predictable. Different shards are independent and can differ. -Different shards can have different configurations since they are independent PostgreSQL clusters. +## Rolling updates -## ConfigMap Contents +Changing the effective config triggers a rolling update of the shard's pods. The operator hashes the +rendered `postgresql.conf` each reconcile and stores it as a pod annotation; when the hash changes, +the pod's spec-hash changes and the operator recreates pods one at a time through the drain state +machine (replicas first, primary last). -The ConfigMap value is plain `postgresql.conf` syntax. pgctld appends it verbatim to its auto-tuned config, so you can include just the params you want to override — there is no template processing and no need to restate auto-tuned defaults. +## Status -### Common Parameters +Each `Shard` reports config rollout state under `status.postgresConfig`, so you can tell whether a +config change has finished rolling out without inspecting pods: -| Parameter | Description | Default | -|:---|:---|:---| -| `shared_buffers` | Shared memory for caching | Auto-tuned by pgctld | -| `work_mem` | Per-operation sort/hash memory | Auto-tuned | -| `max_connections` | Maximum concurrent connections | Auto-tuned | -| `effective_cache_size` | Planner's estimate of OS cache | Auto-tuned | -| `wal_buffers` | WAL write buffer size | Auto-tuned | - -For a complete list of PostgreSQL parameters, see the [PostgreSQL documentation](https://www.postgresql.org/docs/current/runtime-config.html). +```bash +kubectl get shard -o jsonpath='{.status.postgresConfig}' +``` -## Rolling Updates +- `inProgress` — `true` while the desired rendered config has not yet landed on every pool pod + (a config rollout is under way). This signal is **content-based**: the operator compares the + hash of the config effective on the pods against the hash of the desired render, so it reflects + any config change — a spec edit, a `postgresConfigRef` ConfigMap edit, or a new operator baseline + on upgrade — none of which are captured by the shard generation alone. +- `lastAppliedAt` — when the rendered config last settled onto every pool pod. It is (re)stamped + only on the transition into "settled", so it stays stable while nothing is changing and advances + for any change. +- `error` — non-empty when the config could not be rendered or read (e.g. a missing + `postgresConfigRef` ConfigMap). + +The config is **settled** when `inProgress == false && error == ""`. For a spec change +specifically, use the shard's top-level `status.observedGeneration` as the freshness watermark to +confirm the operator has observed your edit: -Changing the referenced ConfigMap's content triggers a rolling update of all pool pods in the shard. The operator computes a SHA-256 hash of the referenced key's data during every reconciliation and stores it as a pod annotation. When the hash changes, the pod's spec-hash changes, and the operator recreates pods one at a time through the drain state machine. +```bash +kubectl get shard -o jsonpath='{.status.observedGeneration}' +``` ## Validation -The operator does not validate the contents of the referenced ConfigMap. PostgreSQL validates the parameters itself when pgctld starts -- invalid parameters will cause the pod to fail at startup with a clear error in the pgctld logs. +The validating webhook checks `postgresConfig` at admission time: each parameter name must be a +known PostgreSQL parameter (or a namespaced extension parameter such as `cron.database_name`), and +the value must roughly match the parameter's type (bool / integer / real). Unknown names and gross +type mismatches are rejected before the resource is accepted, so a typo can't reach the pods. -To debug configuration issues: +The check is deliberately rough — it does not validate every value (for example, specific enum values +or unit correctness). PostgreSQL performs authoritative validation when pgctld starts; an invalid +value that slips through causes the pod to fail at startup with a clear error in the pgctld logs: ```bash kubectl logs -c postgres | grep -i 'error\|invalid\|unrecognized' ``` +The parameter catalog is generated from PostgreSQL 17's `guc_tables.c` and bundled with the operator. + ## Relationship to initdbArgs -| | `postgresConfigRef` | `initdbArgs` | -|:---|:---|:---| -| **When it applies** | Every server start | First initialization only | -| **What it controls** | Runtime PostgreSQL parameters | Data directory initialization options (locale, encoding) | -| **Type** | ConfigMap reference (atomic replacement) | Single string (replacement) | -| **Use case** | Tuning performance, connections, WAL | Setting ICU locale, encoding at init time | +| | `postgresConfig` | `initdbArgs` | +| :------------------- | :------------------------------ | :------------------------------------------------------- | +| **When it applies** | Every server start | First initialization only | +| **What it controls** | Runtime PostgreSQL parameters | Data directory initialization options (locale, encoding) | +| **Type** | Key/value map (per-key merge) | Single string (replacement) | +| **Use case** | Tuning performance, connections | Setting ICU locale, encoding at init time | + +Use `initdbArgs` for one-time initialization options and `postgresConfig` for ongoing runtime tuning. + +## Legacy: postgresConfigRef + +> **Deprecated.** `postgresConfigRef` predates `postgresConfig`. It is still honored — its content is +> merged in just below the inline map (so `postgresConfig` wins on conflicts) — but it will be removed +> in a future version. Prefer `postgresConfig` for new configuration. -Both are shard-level settings with the same override chain. Use `initdbArgs` for one-time initialization options and `postgresConfigRef` for ongoing runtime tuning. +`postgresConfigRef` points at a ConfigMap holding raw `postgresql.conf` lines: -## No Defaulting +```yaml +apiVersion: v1 +kind: ConfigMap +metadata: + name: my-postgres-config +data: + custom.conf: | + shared_buffers = '8GB' + max_connections = 200 +--- +apiVersion: multigres.com/v1alpha1 +kind: MultigresCluster +spec: + databases: + - name: "postgres" + tablegroups: + - name: "default" + shards: + - name: "0-inf" + spec: + postgresConfigRef: + name: my-postgres-config + key: custom.conf +``` -When `postgresConfigRef` is nil (the default), pgctld uses only its auto-tuned values. There is no webhook materialization -- the field stays nil unless you set it. This is intentional: the auto-tuned defaults are appropriate for most workloads. +Unlike the inline map, the reference is an atomic replacement (the whole file is one layer) and +follows **last-non-nil-wins** through the template chain rather than a per-key merge. To migrate, +move each `postgresql.conf` line into the `postgresConfig` map as a key/value pair. diff --git a/pkg/cluster-handler/controller/multigrescluster/reconcile_databases.go b/pkg/cluster-handler/controller/multigrescluster/reconcile_databases.go index f1a59177..ce3c60c2 100644 --- a/pkg/cluster-handler/controller/multigrescluster/reconcile_databases.go +++ b/pkg/cluster-handler/controller/multigrescluster/reconcile_databases.go @@ -64,7 +64,7 @@ func (r *MultigresClusterReconciler) reconcileDatabases( // Pass allCellNames to the resolver so it can perform "Empty means Everybody" defaulting. // tgBackup carries the merged chain: TableGroup -> Database -> Cluster. - orch, pools, pvcPolicy, finalShardBackup, initdbArgs, postgresConfigRef, err := res.ResolveShard( + resolved, err := res.ResolveShard( ctx, shardCfg, resolver.ResolveShardOptions{ @@ -93,12 +93,13 @@ func (r *MultigresClusterReconciler) reconcileDatabases( // We no longer need to manually infer or sort here, just trust the resolver. resolvedShards = append(resolvedShards, multigresv1alpha1.ShardResolvedSpec{ Name: string(shard.Name), - Multiorch: *orch, - InitdbArgs: initdbArgs, - PostgresConfigRef: postgresConfigRef, - Pools: pools, - PVCDeletionPolicy: pvcPolicy, - Backup: finalShardBackup, + Multiorch: resolved.Multiorch, + InitdbArgs: resolved.InitdbArgs, + PostgresConfigRef: resolved.PostgresConfigRef, + PostgresConfig: resolved.PostgresConfig, + Pools: resolved.Pools, + PVCDeletionPolicy: resolved.PVCDeletionPolicy, + Backup: resolved.Backup, }) } diff --git a/pkg/cluster-handler/controller/tablegroup/builders.go b/pkg/cluster-handler/controller/tablegroup/builders.go index 33dff4cf..719a682f 100644 --- a/pkg/cluster-handler/controller/tablegroup/builders.go +++ b/pkg/cluster-handler/controller/tablegroup/builders.go @@ -56,6 +56,7 @@ func BuildShard( Multiorch: shardSpec.Multiorch, InitdbArgs: shardSpec.InitdbArgs, PostgresConfigRef: shardSpec.PostgresConfigRef, + PostgresConfig: shardSpec.PostgresConfig, Pools: shardSpec.Pools, Replicas: calculateTotalReplicas(shardSpec.Pools), // Merge hierarchy: Shard → TableGroup diff --git a/pkg/postgresconfig/catalog/pg_settings_17.txt b/pkg/postgresconfig/catalog/pg_settings_17.txt new file mode 100644 index 00000000..f7bd2c99 --- /dev/null +++ b/pkg/postgresconfig/catalog/pg_settings_17.txt @@ -0,0 +1,382 @@ +allow_alter_system bool +allow_in_place_tablespaces bool +allow_system_table_mods bool +application_name string +archive_cleanup_command string +archive_command string +archive_library string +archive_mode enum +archive_timeout integer +array_nulls bool +authentication_timeout integer +autovacuum bool +autovacuum_analyze_scale_factor real +autovacuum_analyze_threshold integer +autovacuum_freeze_max_age integer +autovacuum_max_workers integer +autovacuum_multixact_freeze_max_age integer +autovacuum_naptime integer +autovacuum_vacuum_cost_delay real +autovacuum_vacuum_cost_limit integer +autovacuum_vacuum_insert_scale_factor real +autovacuum_vacuum_insert_threshold integer +autovacuum_vacuum_scale_factor real +autovacuum_vacuum_threshold integer +autovacuum_work_mem integer +backend_flush_after integer +backslash_quote enum +backtrace_functions string +bgwriter_delay integer +bgwriter_flush_after integer +bgwriter_lru_maxpages integer +bgwriter_lru_multiplier real +block_size integer +bonjour bool +bonjour_name string +bytea_output enum +check_function_bodies bool +checkpoint_completion_target real +checkpoint_flush_after integer +checkpoint_timeout integer +checkpoint_warning integer +client_connection_check_interval integer +client_encoding string +client_min_messages enum +cluster_name string +commit_delay integer +commit_siblings integer +commit_timestamp_buffers integer +compute_query_id enum +config_file string +constraint_exclusion enum +cpu_index_tuple_cost real +cpu_operator_cost real +cpu_tuple_cost real +createrole_self_grant string +cursor_tuple_fraction real +data_checksums bool +data_directory string +data_directory_mode integer +data_sync_retry bool +deadlock_timeout integer +debug_assertions bool +debug_deadlocks bool +debug_discard_caches integer +debug_io_direct string +debug_logical_replication_streaming enum +debug_parallel_query enum +debug_pretty_print bool +debug_print_parse bool +debug_print_plan bool +debug_print_rewritten bool +default_statistics_target integer +default_table_access_method string +default_tablespace string +default_text_search_config string +default_toast_compression enum +default_transaction_deferrable bool +default_transaction_isolation enum +default_transaction_read_only bool +default_with_oids bool +dynamic_library_path string +dynamic_shared_memory_type enum +effective_cache_size integer +enable_async_append bool +enable_bitmapscan bool +enable_gathermerge bool +enable_group_by_reordering bool +enable_hashagg bool +enable_hashjoin bool +enable_incremental_sort bool +enable_indexonlyscan bool +enable_indexscan bool +enable_material bool +enable_memoize bool +enable_mergejoin bool +enable_nestloop bool +enable_parallel_append bool +enable_parallel_hash bool +enable_partition_pruning bool +enable_partitionwise_aggregate bool +enable_partitionwise_join bool +enable_presorted_aggregate bool +enable_seqscan bool +enable_sort bool +enable_tidscan bool +escape_string_warning bool +event_source string +event_triggers bool +exit_on_error bool +external_pid_file string +extra_float_digits integer +from_collapse_limit integer +fsync bool +full_page_writes bool +geqo bool +geqo_effort integer +geqo_generations integer +geqo_pool_size integer +geqo_seed real +geqo_selection_bias real +geqo_threshold integer +gin_fuzzy_search_limit integer +gin_pending_list_limit integer +gss_accept_delegation bool +hash_mem_multiplier real +hba_file string +hot_standby bool +hot_standby_feedback bool +huge_page_size integer +huge_pages enum +huge_pages_status enum +icu_validation_level enum +ident_file string +idle_in_transaction_session_timeout integer +idle_session_timeout integer +ignore_checksum_failure bool +ignore_invalid_pages bool +ignore_system_indexes bool +in_hot_standby bool +integer_datetimes bool +is_superuser bool +jit bool +jit_above_cost real +jit_debugging_support bool +jit_dump_bitcode bool +jit_expressions bool +jit_inline_above_cost real +jit_optimize_above_cost real +jit_profiling_support bool +jit_provider string +jit_tuple_deforming bool +join_collapse_limit integer +krb_caseins_users bool +krb_server_keyfile string +lc_messages string +lc_monetary string +lc_numeric string +lc_time string +listen_addresses string +lo_compat_privileges bool +local_preload_libraries string +lock_timeout integer +log_autovacuum_min_duration integer +log_btree_build_stats bool +log_checkpoints bool +log_connections bool +log_destination string +log_directory string +log_disconnections bool +log_duration bool +log_error_verbosity enum +log_executor_stats bool +log_file_mode integer +log_filename string +log_hostname bool +log_line_prefix string +log_lock_waits bool +log_min_duration_sample integer +log_min_duration_statement integer +log_min_error_statement enum +log_min_messages enum +log_parameter_max_length integer +log_parameter_max_length_on_error integer +log_parser_stats bool +log_planner_stats bool +log_recovery_conflict_waits bool +log_replication_commands bool +log_rotation_age integer +log_rotation_size integer +log_startup_progress_interval integer +log_statement enum +log_statement_sample_rate real +log_statement_stats bool +log_temp_files integer +log_timezone string +log_transaction_sample_rate real +log_truncate_on_rotation bool +logging_collector bool +logical_decoding_work_mem integer +maintenance_work_mem integer +max_connections integer +max_files_per_process integer +max_function_args integer +max_identifier_length integer +max_index_keys integer +max_locks_per_transaction integer +max_notify_queue_pages integer +max_parallel_maintenance_workers integer +max_parallel_workers integer +max_parallel_workers_per_gather integer +max_pred_locks_per_page integer +max_pred_locks_per_relation integer +max_pred_locks_per_transaction integer +max_prepared_transactions integer +max_replication_slots integer +max_slot_wal_keep_size integer +max_stack_depth integer +max_standby_archive_delay integer +max_standby_streaming_delay integer +max_wal_senders integer +max_wal_size integer +min_dynamic_shared_memory integer +min_parallel_index_scan_size integer +min_parallel_table_scan_size integer +min_wal_size integer +multixact_member_buffers integer +multixact_offset_buffers integer +notify_buffers integer +parallel_leader_participation bool +parallel_setup_cost real +parallel_tuple_cost real +password_encryption enum +plan_cache_mode enum +port integer +post_auth_delay integer +pre_auth_delay integer +primary_conninfo string +primary_slot_name string +quote_all_identifiers bool +random_page_cost real +recovery_end_command string +recovery_init_sync_method enum +recovery_min_apply_delay integer +recovery_prefetch enum +recovery_target string +recovery_target_action enum +recovery_target_inclusive bool +recovery_target_lsn string +recovery_target_name string +recovery_target_time string +recovery_target_timeline string +recovery_target_xid string +recursive_worktable_factor real +remove_temp_files_after_crash bool +reserved_connections integer +restart_after_crash bool +restore_command string +restrict_nonsystem_relation_kind string +role string +row_security bool +scram_iterations integer +search_path string +seed real +segment_size integer +send_abort_for_crash bool +send_abort_for_kill bool +seq_page_cost real +serializable_buffers integer +server_encoding string +server_version string +server_version_num integer +session_authorization string +session_preload_libraries string +session_replication_role enum +shared_buffers integer +shared_memory_size integer +shared_memory_size_in_huge_pages integer +shared_memory_type enum +shared_preload_libraries string +ssl bool +ssl_ca_file string +ssl_cert_file string +ssl_ciphers string +ssl_crl_dir string +ssl_crl_file string +ssl_dh_params_file string +ssl_ecdh_curve string +ssl_key_file string +ssl_library string +ssl_max_protocol_version enum +ssl_min_protocol_version enum +ssl_passphrase_command string +ssl_passphrase_command_supports_reload bool +ssl_prefer_server_ciphers bool +ssl_renegotiation_limit integer +standard_conforming_strings bool +statement_timeout integer +stats_fetch_consistency enum +subtransaction_buffers integer +summarize_wal bool +superuser_reserved_connections integer +sync_replication_slots bool +synchronize_seqscans bool +synchronized_standby_slots string +synchronous_commit enum +synchronous_standby_names string +syslog_facility enum +syslog_ident string +syslog_sequence_numbers bool +syslog_split_messages bool +tcp_keepalives_count integer +tcp_keepalives_idle integer +tcp_keepalives_interval integer +tcp_user_timeout integer +temp_buffers integer +temp_file_limit integer +temp_tablespaces string +timezone_abbreviations string +trace_connection_negotiation bool +trace_lock_oidmin integer +trace_lock_table integer +trace_locks bool +trace_lwlocks bool +trace_notify bool +trace_sort bool +trace_syncscan bool +trace_userlocks bool +track_activities bool +track_activity_query_size integer +track_commit_timestamp bool +track_counts bool +track_functions enum +track_io_timing bool +track_wal_io_timing bool +transaction_buffers integer +transaction_deferrable bool +transaction_isolation enum +transaction_read_only bool +transaction_timeout integer +transform_null_equals bool +unix_socket_directories string +unix_socket_group string +unix_socket_permissions integer +update_process_title bool +vacuum_buffer_usage_limit integer +vacuum_cost_delay real +vacuum_cost_limit integer +vacuum_cost_page_dirty integer +vacuum_cost_page_hit integer +vacuum_cost_page_miss integer +vacuum_failsafe_age integer +vacuum_freeze_min_age integer +vacuum_freeze_table_age integer +vacuum_multixact_failsafe_age integer +vacuum_multixact_freeze_min_age integer +vacuum_multixact_freeze_table_age integer +wal_block_size integer +wal_buffers integer +wal_compression enum +wal_consistency_checking string +wal_debug bool +wal_decode_buffer_size integer +wal_init_zero bool +wal_keep_size integer +wal_level enum +wal_log_hints bool +wal_receiver_create_temp_slot bool +wal_receiver_status_interval integer +wal_receiver_timeout integer +wal_recycle bool +wal_retrieve_retry_interval integer +wal_segment_size integer +wal_sender_timeout integer +wal_skip_threshold integer +wal_summary_keep_time integer +wal_sync_method enum +wal_writer_delay integer +wal_writer_flush_after integer +work_mem integer +xmlbinary enum +xmloption enum +zero_damaged_pages bool diff --git a/pkg/postgresconfig/render.go b/pkg/postgresconfig/render.go new file mode 100644 index 00000000..1340205c --- /dev/null +++ b/pkg/postgresconfig/render.go @@ -0,0 +1,135 @@ +// Package postgresconfig renders the effective postgresql.conf the operator +// mounts into pgctld. The operator owns config generation end-to-end: a static +// baseline it defines, followed by the user's legacy PostgresConfigRef content +// and the inline spec.postgresConfig map, each appended so it overrides earlier +// layers (PostgreSQL applies later assignments last-write-wins). Resource- +// derived sizing is baked into the Config before rendering. +package postgresconfig + +import ( + _ "embed" + "fmt" + "sort" + "strings" + "text/template" +) + +// ConfigFileName is the filename the rendered content is projected to inside +// the pod. pgctld reads it via POSTGRES_INITDB_EXTRA_CONF. +const ConfigFileName = "postgresql.conf" + +//go:embed templates/postgresql.conf.tmpl +var baseTemplate string + +var parsedBaseTemplate = template.Must( + template.New("postgresql.conf").Parse(baseTemplate), +) + +// Config holds the tunable postgresql.conf values rendered into the baseline +// template. Non-tunable static lines (SSL, locales, logging, wal_level, ...) +// live in the template itself and need no fields here. +type Config struct { + MaxConnections int + + // Memory settings (postgres size strings, e.g. "128MB"). + SharedBuffers string + MaintenanceWorkMem string + WorkMem string + + // Worker and parallel settings. + MaxWorkerProcesses int + EffectiveIoConcurrency int + MaxParallelWorkers int + MaxParallelWorkersPerGather int + MaxParallelMaintenanceWorkers int + + // WAL settings. + WalBuffers string + MinWalSize string + MaxWalSize string + WalKeepSize string + MaxSlotWalKeepSize string + + // Checkpoint / replication / planner settings. + CheckpointCompletionTarget float64 + MaxWalSenders int + MaxReplicationSlots int + EffectiveCacheSize string + RandomPageCost float64 + DefaultStatisticsTarget int + + ClusterName string +} + +// Defaults returns the operator's built-in static baseline, tuned for a small +// instance. Resource-derived sizing overrides the size-sensitive fields; the +// rest are the shipped baseline. +func Defaults() Config { + return Config{ + MaxConnections: 60, + SharedBuffers: "64MB", + MaintenanceWorkMem: "16MB", + WorkMem: "1092kB", + MaxWorkerProcesses: 6, + EffectiveIoConcurrency: 0, + MaxParallelWorkers: 2, + MaxParallelWorkersPerGather: 1, + MaxParallelMaintenanceWorkers: 1, + WalBuffers: "1920kB", + MinWalSize: "80MB", + MaxWalSize: "1024MB", + WalKeepSize: "1000MB", + MaxSlotWalKeepSize: "1024MB", + CheckpointCompletionTarget: 0.9, + MaxWalSenders: 25, + MaxReplicationSlots: 25, + EffectiveCacheSize: "192MB", + RandomPageCost: 1.1, + DefaultStatisticsTarget: 100, + ClusterName: "default", + } +} + +// Render produces the effective postgresql.conf: the baseline template rendered +// with cfg, followed by the user's legacy PostgresConfigRef content (verbatim) +// and the inline spec.postgresConfig map, each appended so it overrides earlier +// layers. refContent is the body of the user's PostgresConfigRef key, or empty +// when no ref is set; inline may be nil. +func Render(cfg Config, refContent string, inline map[string]string) (string, error) { + var b strings.Builder + if err := parsedBaseTemplate.Execute(&b, cfg); err != nil { + return "", fmt.Errorf("rendering postgres config template: %w", err) + } + + if trimmed := strings.TrimRight(refContent, "\n"); trimmed != "" { + b.WriteString("\n# postgresConfigRef\n") + b.WriteString(trimmed) + b.WriteString("\n") + } + + if len(inline) > 0 { + b.WriteString("\n# spec.postgresConfig\n") + writeSortedMap(&b, inline) + } + + return b.String(), nil +} + +func writeSortedMap(b *strings.Builder, m map[string]string) { + keys := make([]string, 0, len(m)) + for k := range m { + keys = append(keys, k) + } + sort.Strings(keys) + for _, k := range keys { + fmt.Fprintf(b, "%s = %s\n", k, quote(m[k])) + } +} + +// quote wraps a GUC value in single quotes, escaping embedded single quotes by +// doubling them. Single-quoted values are accepted in postgresql.conf for every +// GUC type — numbers, booleans, enums, memory units, and comma-separated lists +// — so this is a safe universal representation regardless of the parameter type. +func quote(v string) string { + return "'" + strings.ReplaceAll(v, "'", "''") + "'" +} diff --git a/pkg/postgresconfig/render_test.go b/pkg/postgresconfig/render_test.go new file mode 100644 index 00000000..9b27f57e --- /dev/null +++ b/pkg/postgresconfig/render_test.go @@ -0,0 +1,129 @@ +package postgresconfig + +import ( + "strings" + "testing" +) + +func TestRender(t *testing.T) { + t.Run("renders the baseline from Config", func(t *testing.T) { + got, err := Render(Defaults(), "", nil) + if err != nil { + t.Fatalf("Render() error = %v", err) + } + // A few representative baseline lines must be present with default values. + for _, want := range []string{ + "max_connections = 60", + "shared_buffers = 64MB", + "effective_cache_size = 192MB", + "wal_level = logical", + "cluster_name = 'default'", + } { + if !strings.Contains(got, want) { + t.Errorf("rendered baseline missing %q, got:\n%s", want, got) + } + } + }) + + t.Run("Config values flow into the template", func(t *testing.T) { + cfg := Defaults() + cfg.SharedBuffers = "2GB" + cfg.MaxConnections = 200 + got, err := Render(cfg, "", nil) + if err != nil { + t.Fatalf("Render() error = %v", err) + } + if !strings.Contains(got, "shared_buffers = 2GB") { + t.Errorf("shared_buffers override missing, got:\n%s", got) + } + if !strings.Contains(got, "max_connections = 200") { + t.Errorf("max_connections override missing, got:\n%s", got) + } + }) + + t.Run("ref content appended verbatim after the baseline", func(t *testing.T) { + ref := "shared_buffers = '8GB'\n# a comment" + got, err := Render(Defaults(), ref, nil) + if err != nil { + t.Fatalf("Render() error = %v", err) + } + if !strings.Contains(got, ref) { + t.Errorf("ref content not emitted verbatim, got:\n%s", got) + } + // Ref must come after the baseline so it wins last-write-wins. + if strings.Index(got, ref) < strings.Index(got, "shared_buffers = 64MB") { + t.Errorf("ref content should follow the baseline, got:\n%s", got) + } + }) + + t.Run("inline map appended last as sorted quoted lines", func(t *testing.T) { + got, err := Render(Defaults(), "ref = 'x'", map[string]string{ + "work_mem": "16MB", + "max_connections": "200", + }) + if err != nil { + t.Fatalf("Render() error = %v", err) + } + wantMax := "max_connections = '200'" + wantWork := "work_mem = '16MB'" + if !strings.Contains(got, wantMax) || !strings.Contains(got, wantWork) { + t.Fatalf("inline lines missing, got:\n%s", got) + } + // Sorted keys, and the whole map block comes after the ref block. + if strings.Index(got, wantMax) > strings.Index(got, wantWork) { + t.Errorf("inline keys not sorted, got:\n%s", got) + } + if strings.Index(got, wantMax) < strings.Index(got, "ref = 'x'") { + t.Errorf("inline map should follow the ref, got:\n%s", got) + } + }) + + t.Run("single quotes in inline values are escaped", func(t *testing.T) { + got, err := Render(Defaults(), "", map[string]string{"log_line_prefix": "it's %m"}) + if err != nil { + t.Fatalf("Render() error = %v", err) + } + if !strings.Contains(got, "log_line_prefix = 'it''s %m'") { + t.Errorf("single quote not escaped, got:\n%s", got) + } + }) + + t.Run("empty ref and nil map render only the baseline", func(t *testing.T) { + got, err := Render(Defaults(), "\n\n", nil) + if err != nil { + t.Fatalf("Render() error = %v", err) + } + if strings.Contains(got, "# postgresConfigRef") || + strings.Contains(got, "# spec.postgresConfig") { + t.Errorf("unexpected override section for empty inputs, got:\n%s", got) + } + }) + + t.Run("deterministic across calls", func(t *testing.T) { + in := map[string]string{"a": "1", "b": "2", "c": "3"} + first, err := Render(Defaults(), "x = 'y'", in) + if err != nil { + t.Fatalf("Render() error = %v", err) + } + second, err := Render(Defaults(), "x = 'y'", in) + if err != nil { + t.Fatalf("Render() error = %v", err) + } + if first != second { + t.Error("Render is not deterministic") + } + }) +} + +func TestDefaults(t *testing.T) { + d := Defaults() + if d.MaxConnections != 60 { + t.Errorf("MaxConnections = %d, want 60", d.MaxConnections) + } + if d.SharedBuffers != "64MB" { + t.Errorf("SharedBuffers = %q, want 64MB", d.SharedBuffers) + } + if d.ClusterName != "default" { + t.Errorf("ClusterName = %q, want default", d.ClusterName) + } +} diff --git a/pkg/postgresconfig/sizing.go b/pkg/postgresconfig/sizing.go new file mode 100644 index 00000000..927986d3 --- /dev/null +++ b/pkg/postgresconfig/sizing.go @@ -0,0 +1,147 @@ +package postgresconfig + +import "fmt" + +const ( + kib = int64(1) << 10 + mib = int64(1) << 20 + gib = int64(1) << 30 + + // parallelWorkerCPUThreshold is the minimum core count before the parallel- + // worker knobs are tuned. Below it the small-instance defaults are kept so + // tiny pods are not starved of worker slots. + parallelWorkerCPUThreshold = 4 + + // maintenanceWorkMemCapBytes caps maintenance_work_mem at 2GB. + maintenanceWorkMemCapBytes = 2 * gib +) + +// ApplyResourceSizing overrides the size-sensitive fields of cfg from resolved +// per-shard resource inputs, mutating cfg in place: +// +// - memory (bytes) and CPU (millicores) drive the memory/worker knobs +// (shared_buffers, effective_cache_size, maintenance_work_mem, work_mem, +// wal_buffers, and the parallel-worker settings); +// - disk (bytes) drives the WAL disk knobs. +// +// Any input that is zero leaves the corresponding baseline defaults untouched, +// so a shard with unset resources keeps the shipped baseline. +func ApplyResourceSizing(cfg *Config, memBytes, cpuMillicores, diskBytes int64) error { + // CPU first: it sets MaxParallelWorkersPerGather, which work_mem divides by. + if cpuMillicores > 0 { + cores := int(cpuMillicores / 1000) + if cores >= parallelWorkerCPUThreshold { + cfg.MaxWorkerProcesses = cores + cfg.MaxParallelWorkers = cores + cfg.MaxParallelWorkersPerGather = cores / 2 + cfg.MaxParallelMaintenanceWorkers = min(cores/2, 4) + } + } + + if memBytes > 0 { + shared := memBytes / 4 + cfg.SharedBuffers = formatBytes(shared) + cfg.EffectiveCacheSize = formatBytes(memBytes * 3 / 4) + cfg.MaintenanceWorkMem = formatBytes(min(memBytes/16, maintenanceWorkMemCapBytes)) + cfg.WalBuffers = formatBytes(clampInt64(shared*3/100, 32*kib, 16*mib)) + + parallel := int64(max(cfg.MaxParallelWorkersPerGather, 1)) + conns := int64(max(cfg.MaxConnections, 1)) + cfg.WorkMem = formatBytes(max((memBytes-shared)/(conns*3)/parallel, 64*kib)) + } + + if diskBytes > 0 { + ws, err := deriveWalSettings(uint64(diskBytes), defaultWalSegmentSizeBytes) + if err != nil { + return err + } + cfg.MinWalSize = fmt.Sprintf("%dMB", ws.minWalSizeMB) + cfg.MaxWalSize = fmt.Sprintf("%dMB", ws.maxWalSizeMB) + cfg.WalKeepSize = fmt.Sprintf("%dMB", ws.walKeepSizeMB) + cfg.MaxSlotWalKeepSize = fmt.Sprintf("%dMB", ws.maxSlotWalKeepSizeMB) + } + return nil +} + +// formatBytes renders a byte count as a postgresql.conf size string using the +// largest unit (GB/MB/kB) that divides it evenly. PostgreSQL's smallest memory +// unit is kB, so sub-kB remainders are truncated. +func formatBytes(b int64) string { + kb := b / kib + if kb <= 0 { + return "0kB" + } + switch { + case kb%(kib*kib) == 0: + return fmt.Sprintf("%dGB", kb/(kib*kib)) + case kb%kib == 0: + return fmt.Sprintf("%dMB", kb/kib) + default: + return fmt.Sprintf("%dkB", kb) + } +} + +func clampInt64(v, lo, hi int64) int64 { + return min(max(v, lo), hi) +} + +// --------------------------------------------------------------------------- +// WAL disk-usage sizing: scale PostgreSQL's WAL disk knobs to the data volume +// so routine WAL retention is budgeted below the disk size. The upper clamps +// keep sane values on large volumes; smaller volumes scale down proportionally. +// PostgreSQL requires min_wal_size and max_wal_size to be at least two WAL +// segments, so the lower clamps also account for the segment size. +// --------------------------------------------------------------------------- + +const ( + megabyte = uint64(1 << 20) + defaultWalSegmentSizeBytes = 16 * megabyte + maxWalSizeCapMB = uint64(4096) +) + +// walSettings holds the WAL disk-usage settings derived from the size of the +// volume backing the data directory, in megabytes. +type walSettings struct { + minWalSizeMB uint64 + maxWalSizeMB uint64 + walKeepSizeMB uint64 + maxSlotWalKeepSizeMB uint64 +} + +func deriveWalSettings(volumeBytes, walSegmentSizeBytes uint64) (walSettings, error) { + if walSegmentSizeBytes == 0 || walSegmentSizeBytes%megabyte != 0 { + return walSettings{}, fmt.Errorf( + "WAL segment size must be a non-zero whole number of megabytes, got %d bytes", + walSegmentSizeBytes, + ) + } + + volMB := volumeBytes / megabyte + walSegmentSizeMB := walSegmentSizeBytes / megabyte + // Keep enough distance between the two PostgreSQL minimums that min_wal_size + // does not consume the entire max_wal_size allowance. + maxWalFloor := max(uint64(64), 4*walSegmentSizeMB) + if maxWalFloor > maxWalSizeCapMB { + return walSettings{}, fmt.Errorf( + "WAL segment size %dMB requires max_wal_size above the supported %dMB cap", + walSegmentSizeMB, maxWalSizeCapMB, + ) + } + minWalFloor := max(uint64(32), 2*walSegmentSizeMB) + maxWal := clampUint64(volMB/4, maxWalFloor, maxWalSizeCapMB) + + return walSettings{ + minWalSizeMB: clampUint64(maxWal/4, minWalFloor, max(uint64(1024), minWalFloor)), + maxWalSizeMB: maxWal, + walKeepSizeMB: clampUint64( + volMB/8, + max(uint64(32), walSegmentSizeMB), + max(uint64(1000), walSegmentSizeMB), + ), + maxSlotWalKeepSizeMB: maxWal, + }, nil +} + +func clampUint64(v, lo, hi uint64) uint64 { + return min(max(v, lo), hi) +} diff --git a/pkg/postgresconfig/sizing_test.go b/pkg/postgresconfig/sizing_test.go new file mode 100644 index 00000000..58cc92e4 --- /dev/null +++ b/pkg/postgresconfig/sizing_test.go @@ -0,0 +1,185 @@ +package postgresconfig + +import "testing" + +func TestApplyResourceSizing_Memory(t *testing.T) { + cfg := Defaults() + // 512Mi memory, no CPU, no disk. + if err := ApplyResourceSizing(&cfg, 512*mib, 0, 0); err != nil { + t.Fatalf("ApplyResourceSizing() error = %v", err) + } + checks := map[string]string{ + "SharedBuffers": "128MB", // 512Mi / 4 + "EffectiveCacheSize": "384MB", // 512Mi * 3/4 + "MaintenanceWorkMem": "32MB", // 512Mi / 16 + "WorkMem": "2184kB", + "WalBuffers": "3932kB", + } + got := map[string]string{ + "SharedBuffers": cfg.SharedBuffers, + "EffectiveCacheSize": cfg.EffectiveCacheSize, + "MaintenanceWorkMem": cfg.MaintenanceWorkMem, + "WorkMem": cfg.WorkMem, + "WalBuffers": cfg.WalBuffers, + } + for k, want := range checks { + if got[k] != want { + t.Errorf("%s = %q, want %q", k, got[k], want) + } + } +} + +func TestApplyResourceSizing_MaintenanceWorkMemCap(t *testing.T) { + cfg := Defaults() + // 64Gi / 16 = 4Gi, which must be capped at 2GB. + if err := ApplyResourceSizing(&cfg, 64*gib, 0, 0); err != nil { + t.Fatalf("ApplyResourceSizing() error = %v", err) + } + if cfg.MaintenanceWorkMem != "2GB" { + t.Errorf("MaintenanceWorkMem = %q, want 2GB (capped)", cfg.MaintenanceWorkMem) + } +} + +func TestApplyResourceSizing_CPU(t *testing.T) { + tests := map[string]struct { + millicores int64 + wantWorker int // MaxWorkerProcesses; 0 means "unchanged from baseline" + wantGather int + wantMaint int + }{ + "below threshold keeps baseline": { + millicores: 3999, + wantWorker: 6, + wantGather: 1, + wantMaint: 1, + }, + "4 cores": { + millicores: 4000, + wantWorker: 4, + wantGather: 2, + wantMaint: 2, + }, + "8 cores caps maintenance at 4": { + millicores: 8000, + wantWorker: 8, + wantGather: 4, + wantMaint: 4, + }, + "16 cores caps maintenance at 4": { + millicores: 16000, + wantWorker: 16, + wantGather: 8, + wantMaint: 4, + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + cfg := Defaults() + if err := ApplyResourceSizing(&cfg, 0, tc.millicores, 0); err != nil { + t.Fatalf("ApplyResourceSizing() error = %v", err) + } + if cfg.MaxWorkerProcesses != tc.wantWorker { + t.Errorf("MaxWorkerProcesses = %d, want %d", cfg.MaxWorkerProcesses, tc.wantWorker) + } + if cfg.MaxParallelWorkersPerGather != tc.wantGather { + t.Errorf( + "MaxParallelWorkersPerGather = %d, want %d", + cfg.MaxParallelWorkersPerGather, + tc.wantGather, + ) + } + if cfg.MaxParallelMaintenanceWorkers != tc.wantMaint { + t.Errorf( + "MaxParallelMaintenanceWorkers = %d, want %d", + cfg.MaxParallelMaintenanceWorkers, + tc.wantMaint, + ) + } + }) + } +} + +func TestApplyResourceSizing_WorkMemUsesParallelWorkers(t *testing.T) { + // With >=4 cores, max_parallel_workers_per_gather rises, which divides + // work_mem down relative to the single-worker case. + single := Defaults() + _ = ApplyResourceSizing(&single, 512*mib, 0, 0) + parallel := Defaults() + _ = ApplyResourceSizing(¶llel, 512*mib, 8000, 0) + if single.WorkMem == parallel.WorkMem { + t.Errorf("work_mem should shrink with more parallel workers: both %q", single.WorkMem) + } +} + +func TestApplyResourceSizing_WAL(t *testing.T) { + cfg := Defaults() + if err := ApplyResourceSizing(&cfg, 0, 0, 1*gib); err != nil { + t.Fatalf("ApplyResourceSizing() error = %v", err) + } + checks := map[string]string{ + "MinWalSize": "64MB", + "MaxWalSize": "256MB", + "WalKeepSize": "128MB", + "MaxSlotWalKeepSize": "256MB", + } + got := map[string]string{ + "MinWalSize": cfg.MinWalSize, + "MaxWalSize": cfg.MaxWalSize, + "WalKeepSize": cfg.WalKeepSize, + "MaxSlotWalKeepSize": cfg.MaxSlotWalKeepSize, + } + for k, want := range checks { + if got[k] != want { + t.Errorf("%s = %q, want %q", k, got[k], want) + } + } +} + +func TestApplyResourceSizing_WALScalesDownOnSmallVolume(t *testing.T) { + small := Defaults() + _ = ApplyResourceSizing(&small, 0, 0, 256*mib) + // 256Mi volume: max_wal_size = clamp(256/4=64, floor 64, cap) = 64MB, well + // below the 1Gi volume's 256MB. + if small.MaxWalSize != "64MB" { + t.Errorf("MaxWalSize for 256Mi volume = %q, want 64MB", small.MaxWalSize) + } +} + +func TestApplyResourceSizing_ZeroInputsLeaveBaseline(t *testing.T) { + cfg := Defaults() + base := Defaults() + if err := ApplyResourceSizing(&cfg, 0, 0, 0); err != nil { + t.Fatalf("ApplyResourceSizing() error = %v", err) + } + if cfg != base { + t.Errorf("zero inputs mutated the config: %+v != %+v", cfg, base) + } +} + +func TestFormatBytes(t *testing.T) { + tests := map[int64]string{ + 2 * gib: "2GB", + 128 * mib: "128MB", + 64 * kib: "64kB", + 1500: "1kB", // sub-kB remainder truncated + 0: "0kB", + } + for in, want := range tests { + if got := formatBytes(in); got != want { + t.Errorf("formatBytes(%d) = %q, want %q", in, got, want) + } + } +} + +func TestDeriveWalSettings_Errors(t *testing.T) { + if _, err := deriveWalSettings(1*uint64(gib), 0); err == nil { + t.Error("expected error for zero WAL segment size") + } + if _, err := deriveWalSettings(1*uint64(gib), megabyte+1); err == nil { + t.Error("expected error for non-MB-aligned WAL segment size") + } + // A WAL segment large enough that its floor exceeds the max_wal_size cap. + if _, err := deriveWalSettings(1*uint64(gib), 2048*megabyte); err == nil { + t.Error("expected error when segment size forces max_wal_size above the cap") + } +} diff --git a/pkg/postgresconfig/templates/postgresql.conf.tmpl b/pkg/postgresconfig/templates/postgresql.conf.tmpl new file mode 100644 index 00000000..f5898edf --- /dev/null +++ b/pkg/postgresconfig/templates/postgresql.conf.tmpl @@ -0,0 +1,113 @@ +# PostgreSQL configuration rendered by multigres-operator. +# port, listen_addresses, unix_socket_directories, data_directory, hba_file and +# ident_file are intentionally omitted: pgctld pins them via command-line args. + +#------------------------------------------------------------------------------ +# CONNECTIONS AND AUTHENTICATION +#------------------------------------------------------------------------------ + +max_connections = {{.MaxConnections}} + +authentication_timeout = 1min # 1s-600s +password_encryption = scram-sha-256 # scram-sha-256 or md5 + +# - SSL - +ssl = off +ssl_ca_file = '' +ssl_cert_file = '' +ssl_crl_file = '' +ssl_crl_dir = '' +ssl_key_file = '' +ssl_ciphers = 'HIGH:MEDIUM:+3DES:!aNULL' # allowed SSL ciphers +ssl_prefer_server_ciphers = on +ssl_ecdh_curve = 'prime256v1' +ssl_min_protocol_version = 'TLSv1.2' +ssl_max_protocol_version = '' +ssl_dh_params_file = '' +ssl_passphrase_command = '' +ssl_passphrase_command_supports_reload = off + +#------------------------------------------------------------------------------ +# RESOURCE USAGE (except WAL) +#------------------------------------------------------------------------------ + +# - Memory - +shared_buffers = {{.SharedBuffers}} # min 128kB (change requires restart) +maintenance_work_mem = {{.MaintenanceWorkMem}} # min 1MB +work_mem = {{.WorkMem}} # min 64kB + +# - Kernel Resources - +max_worker_processes = {{.MaxWorkerProcesses}} # (change requires restart) + +# - Asynchronous Behavior - +effective_io_concurrency = {{.EffectiveIoConcurrency}} # 1-1000; 0 disables prefetching +max_parallel_workers = {{.MaxParallelWorkers}} # max number of parallel workers +max_parallel_workers_per_gather = {{.MaxParallelWorkersPerGather}} +max_parallel_maintenance_workers = {{.MaxParallelMaintenanceWorkers}} + +#------------------------------------------------------------------------------ +# WRITE-AHEAD LOG +#------------------------------------------------------------------------------ + +wal_level = logical # minimal, replica, or logical (change requires restart) +wal_buffers = {{.WalBuffers}} # min 32kB (change requires restart) +min_wal_size = {{.MinWalSize}} +max_wal_size = {{.MaxWalSize}} +wal_keep_size = {{.WalKeepSize}} + +# - Checkpoints - +checkpoint_completion_target = {{.CheckpointCompletionTarget}} # 0.0 - 1.0 +checkpoint_flush_after = 256kB # measured in pages, 0 disables + +#------------------------------------------------------------------------------ +# REPLICATION +#------------------------------------------------------------------------------ + +max_wal_senders = {{.MaxWalSenders}} # (change requires restart) +max_replication_slots = {{.MaxReplicationSlots}} # (change requires restart) +# Protect the primary's disk; a logical consumer that exceeds this cap can +# lose its slot at checkpoint and require resynchronization. +max_slot_wal_keep_size = {{.MaxSlotWalKeepSize}} # in megabytes; -1 disables + +#------------------------------------------------------------------------------ +# QUERY TUNING +#------------------------------------------------------------------------------ + +effective_cache_size = {{.EffectiveCacheSize}} +random_page_cost = {{.RandomPageCost}} # same scale as above +default_statistics_target = {{.DefaultStatisticsTarget}} # range 1-10000 + +#------------------------------------------------------------------------------ +# REPORTING AND LOGGING +#------------------------------------------------------------------------------ + +log_line_prefix = '%h %m [%p] %q%u@%d ' +log_statement = 'ddl' # none, ddl, mod, all +log_timezone = 'UTC' + +#------------------------------------------------------------------------------ +# PROCESS TITLE +#------------------------------------------------------------------------------ + +cluster_name = '{{.ClusterName}}' # added to process titles if nonempty (change requires restart) + +#------------------------------------------------------------------------------ +# CLIENT CONNECTION DEFAULTS +#------------------------------------------------------------------------------ + +row_security = on +timezone = 'UTC' +lc_messages = 'en_US.UTF-8' +lc_monetary = 'en_US.UTF-8' +lc_numeric = 'en_US.UTF-8' +lc_time = 'en_US.UTF-8' +default_text_search_config = 'pg_catalog.english' +jit_provider = 'llvmjit' # JIT library to use + +#------------------------------------------------------------------------------ +# CUSTOMIZED OPTIONS +#------------------------------------------------------------------------------ + +auto_explain.log_min_duration = 10s +cron.database_name = 'postgres' +wal_log_hints = 'on' diff --git a/pkg/postgresconfig/validate.go b/pkg/postgresconfig/validate.go new file mode 100644 index 00000000..ec4dce11 --- /dev/null +++ b/pkg/postgresconfig/validate.go @@ -0,0 +1,170 @@ +package postgresconfig + +import ( + "bufio" + _ "embed" + "fmt" + "sort" + "strconv" + "strings" +) + +// pgSettingsCatalog maps each built-in PostgreSQL parameter to its broad type. +// It is generated from PostgreSQL's src/backend/utils/misc/guc_tables.c +// (REL_17_5, matching the operator's default Postgres image). Regenerate for a +// new major version by re-extracting name+type from that file. +// +//go:embed catalog/pg_settings_17.txt +var pgSettingsCatalog string + +// gucType is the broad PostgreSQL parameter type used for rough value checks. +type gucType string + +const ( + gucBool gucType = "bool" + gucInteger gucType = "integer" + gucReal gucType = "real" + gucString gucType = "string" + gucEnum gucType = "enum" +) + +var catalog = parseCatalog(pgSettingsCatalog) + +// managedGUCs are parameters the operator owns; a user override in +// spec.postgresConfig is rejected rather than silently mis-served. Two reasons: +// +// - pgctld pins the connection/path parameters on the postgres command line, +// which wins over any postgresql.conf line, so a user value would be +// silently ignored — a confusing no-op rather than an honest rejection. +// - wal_level must stay "logical": multigres' replication and CDC depend on +// it, and lowering it (once the mounted config is re-read on restart) breaks +// the control plane. +// +// The value is the reason surfaced in the admission error. +var managedGUCs = map[string]string{ + "port": "pinned by the operator on the pgctld command line", + "listen_addresses": "pinned by the operator on the pgctld command line", + "unix_socket_directories": "pinned by the operator on the pgctld command line", + "data_directory": "pinned by the operator on the pgctld command line", + "hba_file": "pinned by the operator on the pgctld command line", + "ident_file": "pinned by the operator on the pgctld command line", + "wal_level": "managed by the operator; multigres requires wal_level=logical", +} + +func parseCatalog(data string) map[string]gucType { + m := make(map[string]gucType) + sc := bufio.NewScanner(strings.NewReader(data)) + for sc.Scan() { + name, typ, ok := strings.Cut(strings.TrimSpace(sc.Text()), "\t") + if !ok { + continue + } + m[name] = gucType(typ) + } + return m +} + +// Validate checks each parameter in a spec.postgresConfig map: the name must be +// a known PostgreSQL parameter (or a namespaced extension parameter such as +// "cron.database_name"), and the value must roughly match the parameter's type. +// It returns a single error listing every problem it finds, or nil. +// +// Validation is deliberately rough: it catches unknown names and gross type +// mismatches (e.g. a bool value for an integer parameter), not every invalid +// value. PostgreSQL performs authoritative validation when the server starts. +func Validate(cfg map[string]string) error { + if len(cfg) == 0 { + return nil + } + + names := make([]string, 0, len(cfg)) + for k := range cfg { + names = append(names, k) + } + sort.Strings(names) // deterministic error messages + + var problems []string + for _, name := range names { + if err := validateGUC(name, cfg[name]); err != nil { + problems = append(problems, err.Error()) + } + } + if len(problems) > 0 { + return fmt.Errorf("invalid postgresConfig: %s", strings.Join(problems, "; ")) + } + return nil +} + +func validateGUC(name, value string) error { + // Namespaced (extension) parameters like "cron.database_name" are custom + // placeholders PostgreSQL accepts even without the extension loaded, so their + // names and values cannot be checked against the built-in catalog. + if strings.Contains(name, ".") { + return nil + } + // PostgreSQL parameter names are case-insensitive; the catalog is lower-case. + lower := strings.ToLower(strings.TrimSpace(name)) + if reason, managed := managedGUCs[lower]; managed { + return fmt.Errorf( + "parameter %q is managed by the operator and cannot be set: %s", + name, + reason, + ) + } + typ, ok := catalog[lower] + if !ok { + return fmt.Errorf("unknown parameter %q", name) + } + if !valueMatchesType(value, typ) { + return fmt.Errorf("parameter %q expects a %s value, got %q", name, typ, value) + } + return nil +} + +func valueMatchesType(value string, typ gucType) bool { + v := strings.TrimSpace(value) + switch typ { + case gucBool: + switch strings.ToLower(v) { + case "on", "off", "true", "false", "yes", "no", "1", "0": + return true + } + return false + case gucInteger: + return isIntegerWithOptionalUnit(v) + case gucReal: + _, err := strconv.ParseFloat(v, 64) + return err == nil + case gucEnum, gucString: + // Rough: the name is verified against the catalog; specific enum/string + // values are left to PostgreSQL to validate at startup. + return true + } + return true +} + +// isIntegerWithOptionalUnit accepts an integer optionally followed by a memory +// or time unit (e.g. "200", "128MB", "5min"). It does not verify the unit is +// valid for the specific parameter — PostgreSQL does that at startup. +func isIntegerWithOptionalUnit(v string) bool { + v = strings.TrimSpace(v) + i := 0 + if i < len(v) && (v[i] == '-' || v[i] == '+') { + i++ + } + digits := 0 + for i < len(v) && v[i] >= '0' && v[i] <= '9' { + i++ + digits++ + } + if digits == 0 { + return false + } + unit := strings.TrimSpace(v[i:]) + for _, r := range unit { + if (r < 'a' || r > 'z') && (r < 'A' || r > 'Z') { + return false + } + } + return true +} diff --git a/pkg/postgresconfig/validate_test.go b/pkg/postgresconfig/validate_test.go new file mode 100644 index 00000000..b808050d --- /dev/null +++ b/pkg/postgresconfig/validate_test.go @@ -0,0 +1,125 @@ +package postgresconfig + +import ( + "strings" + "testing" +) + +func TestValidate_Accepts(t *testing.T) { + tests := map[string]map[string]string{ + "empty is fine": nil, + "known integer, plain": {"max_connections": "200"}, + "known integer with unit": {"shared_buffers": "128MB"}, + "known integer, time unit": {"statement_timeout": "5min"}, + "known bool forms": { + "fsync": "on", + "wal_log_hints": "true", + "hot_standby": "0", + }, + "known real": {"random_page_cost": "1.1"}, + "known enum (value not checked)": {"log_statement": "ddl"}, + "known string": {"log_line_prefix": "%m [%p] "}, + "case-insensitive name": {"Max_Connections": "200"}, + "namespaced extension param": { + "cron.database_name": "postgres", + "auto_explain.log_min_duration": "10s", + }, + } + for name, cfg := range tests { + t.Run(name, func(t *testing.T) { + if err := Validate(cfg); err != nil { + t.Errorf("Validate(%v) = %v, want nil", cfg, err) + } + }) + } +} + +func TestValidate_Rejects(t *testing.T) { + tests := map[string]struct { + cfg map[string]string + wantSubs []string + }{ + "unknown name": { + cfg: map[string]string{"maxx_connections": "200"}, + wantSubs: []string{"unknown parameter", "maxx_connections"}, + }, + "bool value for integer": { + cfg: map[string]string{"max_connections": "yes"}, + wantSubs: []string{"max_connections", "integer"}, + }, + "non-bool for bool": { + cfg: map[string]string{"fsync": "maybe"}, + wantSubs: []string{"fsync", "bool"}, + }, + "non-numeric for real": { + cfg: map[string]string{"random_page_cost": "cheap"}, + wantSubs: []string{"random_page_cost", "real"}, + }, + "fractional for integer": { + cfg: map[string]string{"max_connections": "1.5"}, + wantSubs: []string{"max_connections", "integer"}, + }, + "operator-managed wal_level": { + cfg: map[string]string{"wal_level": "replica"}, + wantSubs: []string{"wal_level", "managed by the operator"}, + }, + "operator-managed connection param": { + cfg: map[string]string{"listen_addresses": "127.0.0.1"}, + wantSubs: []string{"listen_addresses", "managed by the operator"}, + }, + "operator-managed, case-insensitive": { + cfg: map[string]string{"WAL_LEVEL": "minimal"}, + wantSubs: []string{"managed by the operator"}, + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + err := Validate(tc.cfg) + if err == nil { + t.Fatalf("Validate(%v) = nil, want error", tc.cfg) + } + for _, sub := range tc.wantSubs { + if !strings.Contains(err.Error(), sub) { + t.Errorf("error %q missing %q", err.Error(), sub) + } + } + }) + } +} + +func TestValidate_AggregatesAllProblems(t *testing.T) { + err := Validate(map[string]string{ + "maxx_connections": "200", // unknown + "fsync": "maybe", // bad bool + "max_connections": "200", // valid — should not appear + }) + if err == nil { + t.Fatal("expected error") + } + if !strings.Contains(err.Error(), "maxx_connections") || + !strings.Contains(err.Error(), "fsync") { + t.Errorf("error should mention both problems: %v", err) + } + if strings.Contains(err.Error(), `"max_connections"`) { + t.Errorf("error should not flag the valid parameter: %v", err) + } +} + +func TestCatalogLoaded(t *testing.T) { + // The embedded catalog must be non-trivial and contain well-known params. + if len(catalog) < 300 { + t.Errorf("catalog has %d entries, expected the full PG17 set (~382)", len(catalog)) + } + for name, want := range map[string]gucType{ + "max_connections": gucInteger, + "shared_buffers": gucInteger, + "fsync": gucBool, + "random_page_cost": gucReal, + "wal_level": gucEnum, + "log_line_prefix": gucString, + } { + if got := catalog[name]; got != want { + t.Errorf("catalog[%q] = %q, want %q", name, got, want) + } + } +} diff --git a/pkg/resolver/shard.go b/pkg/resolver/shard.go index 9146e7d9..18baf782 100644 --- a/pkg/resolver/shard.go +++ b/pkg/resolver/shard.go @@ -19,21 +19,35 @@ type ResolveShardOptions struct { MaterializeCellDefaults bool } +// ResolvedShard is the fully merged and defaulted configuration for a single +// Shard, produced by ResolveShard. Grouping the fields in a struct keeps call +// sites self-documenting and lets new config layers be added without changing +// every signature and call site in the resolve chain. +type ResolvedShard struct { + Multiorch multigresv1alpha1.MultiorchSpec + Pools map[multigresv1alpha1.PoolName]multigresv1alpha1.PoolSpec + PVCDeletionPolicy *multigresv1alpha1.PVCDeletionPolicy + Backup *multigresv1alpha1.BackupConfig + InitdbArgs multigresv1alpha1.InitdbArgs + PostgresConfigRef *multigresv1alpha1.PostgresConfigRef + PostgresConfig map[string]string +} + // ResolveShard determines the final configuration for a specific Shard. func (r *Resolver) ResolveShard( ctx context.Context, shardSpec *multigresv1alpha1.ShardConfig, opts ResolveShardOptions, -) (*multigresv1alpha1.MultiorchSpec, map[multigresv1alpha1.PoolName]multigresv1alpha1.PoolSpec, *multigresv1alpha1.PVCDeletionPolicy, *multigresv1alpha1.BackupConfig, multigresv1alpha1.InitdbArgs, *multigresv1alpha1.PostgresConfigRef, error) { +) (*ResolvedShard, error) { // 1. Fetch Template templateName := shardSpec.ShardTemplate tpl, err := r.ResolveShardTemplate(ctx, templateName) if err != nil { - return nil, nil, nil, nil, "", nil, err + return nil, err } // 2. Merge Logic - multiorch, pools, pvcPolicy, backupCfg, initdbArgs, postgresConfigRef := mergeShardConfig( + res := mergeShardConfig( tpl, shardSpec.Overrides, shardSpec.Spec, @@ -42,35 +56,35 @@ func (r *Resolver) ResolveShard( ) // 3. Apply Deep Defaults (Level 4) - defaultStatelessSpec(&multiorch.StatelessSpec, DefaultResourcesOrch(), 1) + defaultStatelessSpec(&res.Multiorch.StatelessSpec, DefaultResourcesOrch(), 1) - if backupCfg == nil { - backupCfg = &multigresv1alpha1.BackupConfig{ + if res.Backup == nil { + res.Backup = &multigresv1alpha1.BackupConfig{ Type: multigresv1alpha1.BackupTypeFilesystem, } } - defaultBackupConfig(backupCfg) + defaultBackupConfig(res.Backup) // Contextual Defaulting: Lazy Cell Injection // If the resolved configuration has no cells defined, it means "run everywhere". // We inject the full list of cluster cells here. - if opts.MaterializeCellDefaults && len(multiorch.Cells) == 0 && len(opts.AllCellNames) > 0 { + if opts.MaterializeCellDefaults && len(res.Multiorch.Cells) == 0 && len(opts.AllCellNames) > 0 { for _, c := range opts.AllCellNames { - multiorch.Cells = append(multiorch.Cells, multigresv1alpha1.CellName(c)) + res.Multiorch.Cells = append(res.Multiorch.Cells, multigresv1alpha1.CellName(c)) } // Sort for deterministic output - slices.Sort(multiorch.Cells) + slices.Sort(res.Multiorch.Cells) } - if len(pools) == 0 { - pools[DefaultPoolName] = multigresv1alpha1.PoolSpec{ + if len(res.Pools) == 0 { + res.Pools[DefaultPoolName] = multigresv1alpha1.PoolSpec{ Type: "readWrite", - Cells: multiorch.Cells, + Cells: res.Multiorch.Cells, } } - for name := range pools { - p := pools[name] + for name := range res.Pools { + p := res.Pools[name] // Contextual Defaulting for Pools if opts.MaterializeCellDefaults && len(p.Cells) == 0 && len(opts.AllCellNames) > 0 { @@ -100,10 +114,10 @@ func (r *Resolver) ResolveShard( } defaultPoolSpec(&p) - pools[name] = p + res.Pools[name] = p } - return &multiorch, pools, pvcPolicy, backupCfg, initdbArgs, postgresConfigRef, nil + return res, nil } // ResolveShardTemplate fetches and resolves a ShardTemplate by name. @@ -148,89 +162,107 @@ func mergeShardConfig( inline *multigresv1alpha1.ShardInlineSpec, backupOverride *multigresv1alpha1.BackupConfig, inheritedBackup *multigresv1alpha1.BackupConfig, -) (multigresv1alpha1.MultiorchSpec, map[multigresv1alpha1.PoolName]multigresv1alpha1.PoolSpec, *multigresv1alpha1.PVCDeletionPolicy, *multigresv1alpha1.BackupConfig, multigresv1alpha1.InitdbArgs, *multigresv1alpha1.PostgresConfigRef) { +) *ResolvedShard { // 1. Start with Template (Base) - var multiorch multigresv1alpha1.MultiorchSpec - pools := make(map[multigresv1alpha1.PoolName]multigresv1alpha1.PoolSpec) - var pvcPolicy *multigresv1alpha1.PVCDeletionPolicy - var initdbArgs multigresv1alpha1.InitdbArgs - var postgresConfigRef *multigresv1alpha1.PostgresConfigRef + res := &ResolvedShard{ + Pools: make(map[multigresv1alpha1.PoolName]multigresv1alpha1.PoolSpec), + } // Start with inherited backup as base - var backupCfg *multigresv1alpha1.BackupConfig if inheritedBackup != nil { - backupCfg = inheritedBackup.DeepCopy() + res.Backup = inheritedBackup.DeepCopy() } if template != nil { if template.Spec.Multiorch != nil { - multiorch = *template.Spec.Multiorch.DeepCopy() + res.Multiorch = *template.Spec.Multiorch.DeepCopy() } for k, v := range template.Spec.Pools { - pools[k] = *v.DeepCopy() + res.Pools[k] = *v.DeepCopy() } if template.Spec.PVCDeletionPolicy != nil { - pvcPolicy = template.Spec.PVCDeletionPolicy + res.PVCDeletionPolicy = template.Spec.PVCDeletionPolicy } - initdbArgs = template.Spec.InitdbArgs + res.InitdbArgs = template.Spec.InitdbArgs if template.Spec.PostgresConfigRef != nil { - postgresConfigRef = template.Spec.PostgresConfigRef + res.PostgresConfigRef = template.Spec.PostgresConfigRef } + res.PostgresConfig = mergePostgresConfig(res.PostgresConfig, template.Spec.PostgresConfig) } // 2. Apply Overrides (Explicit Template Modification) if overrides != nil { if overrides.Multiorch != nil { - mergeMultiorchSpec(&multiorch, overrides.Multiorch) + mergeMultiorchSpec(&res.Multiorch, overrides.Multiorch) } for k, v := range overrides.Pools { - if existingPool, exists := pools[k]; exists { - pools[k] = mergePoolSpec(existingPool, v) + if existingPool, exists := res.Pools[k]; exists { + res.Pools[k] = mergePoolSpec(existingPool, v) } else { - pools[k] = v + res.Pools[k] = v } } if overrides.InitdbArgs != "" { - initdbArgs = overrides.InitdbArgs + res.InitdbArgs = overrides.InitdbArgs } if overrides.PostgresConfigRef != nil { - postgresConfigRef = overrides.PostgresConfigRef + res.PostgresConfigRef = overrides.PostgresConfigRef } + res.PostgresConfig = mergePostgresConfig(res.PostgresConfig, overrides.PostgresConfig) } // 3. Apply Inline Spec (Primary Overlay) // This merges the inline definition on top of the template+overrides. if inline != nil { - mergeMultiorchSpec(&multiorch, &inline.Multiorch) + mergeMultiorchSpec(&res.Multiorch, &inline.Multiorch) for k, v := range inline.Pools { - if existingPool, exists := pools[k]; exists { - pools[k] = mergePoolSpec(existingPool, v) + if existingPool, exists := res.Pools[k]; exists { + res.Pools[k] = mergePoolSpec(existingPool, v) } else { - pools[k] = v + res.Pools[k] = v } } // Inline PVCDeletionPolicy overrides template if inline.PVCDeletionPolicy != nil { - pvcPolicy = inline.PVCDeletionPolicy + res.PVCDeletionPolicy = inline.PVCDeletionPolicy } if inline.InitdbArgs != "" { - initdbArgs = inline.InitdbArgs + res.InitdbArgs = inline.InitdbArgs } if inline.PostgresConfigRef != nil { - postgresConfigRef = inline.PostgresConfigRef + res.PostgresConfigRef = inline.PostgresConfigRef } + res.PostgresConfig = mergePostgresConfig(res.PostgresConfig, inline.PostgresConfig) } // 4. Apply Backup Override (from ShardConfig.Backup) // We use MergeBackupConfig so that ShardConfig overrides inherited config if backupOverride != nil { - backupCfg = multigresv1alpha1.MergeBackupConfig(backupOverride, backupCfg) + res.Backup = multigresv1alpha1.MergeBackupConfig(backupOverride, res.Backup) } - return multiorch, pools, pvcPolicy, backupCfg, initdbArgs, postgresConfigRef + return res +} + +// mergePostgresConfig overlays override onto base per key (override wins on +// conflicts) and returns the merged map. It returns nil when the result would +// be empty so an unused field stays nil through the resolve chain, matching how +// PostgresConfigRef stays nil when unset. The inputs are never mutated. +func mergePostgresConfig(base, override map[string]string) map[string]string { + if len(base) == 0 && len(override) == 0 { + return nil + } + out := make(map[string]string, len(base)+len(override)) + for k, v := range base { + out[k] = v + } + for k, v := range override { + out[k] = v + } + return out } func mergeMultiorchSpec( diff --git a/pkg/resolver/shard_test.go b/pkg/resolver/shard_test.go index 549164b8..7d71d7b1 100644 --- a/pkg/resolver/shard_test.go +++ b/pkg/resolver/shard_test.go @@ -320,7 +320,7 @@ func TestResolver_ResolveShard(t *testing.T) { c := fake.NewClientBuilder().WithScheme(scheme).WithObjects(tc.objects...).Build() r := NewResolver(c, ns) - orch, pools, pvcPolicy, _, _, _, err := r.ResolveShard( + resolved, err := r.ResolveShard( t.Context(), tc.config, ResolveShardOptions{ @@ -337,6 +337,7 @@ func TestResolver_ResolveShard(t *testing.T) { if err != nil { t.Fatalf("Unexpected error: %v", err) } + orch, pools, pvcPolicy := &resolved.Multiorch, resolved.Pools, resolved.PVCDeletionPolicy if diff := cmp.Diff( tc.wantOrch, @@ -502,7 +503,7 @@ func TestMergeShardConfig_RuntimeIdentityPartialOverride(t *testing.T) { t.Run("override multipooler UID can match template postgres UID", func(t *testing.T) { t.Parallel() - _, pools, _, _, _, _ := mergeShardConfig( + resolved := mergeShardConfig( &multigresv1alpha1.ShardTemplate{ Spec: multigresv1alpha1.ShardTemplateSpec{ Pools: map[multigresv1alpha1.PoolName]multigresv1alpha1.PoolSpec{ @@ -528,7 +529,7 @@ func TestMergeShardConfig_RuntimeIdentityPartialOverride(t *testing.T) { nil, ) - got := pools["rw"] + got := resolved.Pools["rw"] if got.Postgres.RunAsUser == nil || *got.Postgres.RunAsUser != 1000 { t.Fatalf("postgres runAsUser = %v, want 1000", got.Postgres.RunAsUser) } @@ -540,7 +541,7 @@ func TestMergeShardConfig_RuntimeIdentityPartialOverride(t *testing.T) { t.Run("mismatched override remains visible for resolved validation", func(t *testing.T) { t.Parallel() - _, pools, _, _, _, _ := mergeShardConfig( + resolved := mergeShardConfig( &multigresv1alpha1.ShardTemplate{ Spec: multigresv1alpha1.ShardTemplateSpec{ Pools: map[multigresv1alpha1.PoolName]multigresv1alpha1.PoolSpec{ @@ -566,7 +567,7 @@ func TestMergeShardConfig_RuntimeIdentityPartialOverride(t *testing.T) { nil, ) - got := pools["rw"] + got := resolved.Pools["rw"] if got.Postgres.RunAsUser == nil || *got.Postgres.RunAsUser != 1000 { t.Fatalf("postgres runAsUser = %v, want 1000", got.Postgres.RunAsUser) } @@ -987,13 +988,14 @@ func TestMergeShardConfig(t *testing.T) { for name, tc := range tests { t.Run(name, func(t *testing.T) { t.Parallel() - orch, pools, _, _, _, _ := mergeShardConfig( + resolved := mergeShardConfig( tc.tpl, tc.overrides, tc.inline, nil, nil, ) + orch, pools := resolved.Multiorch, resolved.Pools if diff := cmp.Diff( tc.wantOrch, @@ -1019,14 +1021,14 @@ func TestMergeShardConfig_InitdbArgs(t *testing.T) { t.Run("template sets InitdbArgs", func(t *testing.T) { t.Parallel() - _, _, _, _, initdbArgs, _ := mergeShardConfig( + initdbArgs := mergeShardConfig( &multigresv1alpha1.ShardTemplate{ Spec: multigresv1alpha1.ShardTemplateSpec{ InitdbArgs: "--locale-provider=icu", }, }, nil, nil, nil, nil, - ) + ).InitdbArgs if initdbArgs != "--locale-provider=icu" { t.Errorf("initdbArgs = %q, want %q", initdbArgs, "--locale-provider=icu") } @@ -1034,7 +1036,7 @@ func TestMergeShardConfig_InitdbArgs(t *testing.T) { t.Run("overrides override template", func(t *testing.T) { t.Parallel() - _, _, _, _, initdbArgs, _ := mergeShardConfig( + initdbArgs := mergeShardConfig( &multigresv1alpha1.ShardTemplate{ Spec: multigresv1alpha1.ShardTemplateSpec{ InitdbArgs: "--locale-provider=icu", @@ -1044,7 +1046,7 @@ func TestMergeShardConfig_InitdbArgs(t *testing.T) { InitdbArgs: "--data-checksums", }, nil, nil, nil, - ) + ).InitdbArgs if initdbArgs != "--data-checksums" { t.Errorf("initdbArgs = %q, want %q", initdbArgs, "--data-checksums") } @@ -1052,7 +1054,7 @@ func TestMergeShardConfig_InitdbArgs(t *testing.T) { t.Run("inline overrides template", func(t *testing.T) { t.Parallel() - _, _, _, _, initdbArgs, _ := mergeShardConfig( + initdbArgs := mergeShardConfig( &multigresv1alpha1.ShardTemplate{ Spec: multigresv1alpha1.ShardTemplateSpec{ InitdbArgs: "--locale-provider=icu", @@ -1063,7 +1065,7 @@ func TestMergeShardConfig_InitdbArgs(t *testing.T) { InitdbArgs: "--data-checksums", }, nil, nil, - ) + ).InitdbArgs if initdbArgs != "--data-checksums" { t.Errorf("initdbArgs = %q, want %q", initdbArgs, "--data-checksums") } @@ -1071,7 +1073,7 @@ func TestMergeShardConfig_InitdbArgs(t *testing.T) { t.Run("inline overrides both template and overrides", func(t *testing.T) { t.Parallel() - _, _, _, _, initdbArgs, _ := mergeShardConfig( + initdbArgs := mergeShardConfig( &multigresv1alpha1.ShardTemplate{ Spec: multigresv1alpha1.ShardTemplateSpec{ InitdbArgs: "--locale-provider=icu", @@ -1084,7 +1086,7 @@ func TestMergeShardConfig_InitdbArgs(t *testing.T) { InitdbArgs: "--wal-segsize=64", }, nil, nil, - ) + ).InitdbArgs if initdbArgs != "--wal-segsize=64" { t.Errorf("initdbArgs = %q, want %q", initdbArgs, "--wal-segsize=64") } @@ -1092,10 +1094,10 @@ func TestMergeShardConfig_InitdbArgs(t *testing.T) { t.Run("no InitdbArgs anywhere", func(t *testing.T) { t.Parallel() - _, _, _, _, initdbArgs, _ := mergeShardConfig( + initdbArgs := mergeShardConfig( &multigresv1alpha1.ShardTemplate{}, nil, nil, nil, nil, - ) + ).InitdbArgs if initdbArgs != "" { t.Errorf("initdbArgs = %q, want empty", initdbArgs) } @@ -1103,7 +1105,7 @@ func TestMergeShardConfig_InitdbArgs(t *testing.T) { t.Run("empty override does not clear template value", func(t *testing.T) { t.Parallel() - _, _, _, _, initdbArgs, _ := mergeShardConfig( + initdbArgs := mergeShardConfig( &multigresv1alpha1.ShardTemplate{ Spec: multigresv1alpha1.ShardTemplateSpec{ InitdbArgs: "--locale-provider=icu", @@ -1113,7 +1115,7 @@ func TestMergeShardConfig_InitdbArgs(t *testing.T) { InitdbArgs: "", }, nil, nil, nil, - ) + ).InitdbArgs if initdbArgs != "--locale-provider=icu" { t.Errorf("initdbArgs = %q, want %q (empty override should not clear template)", initdbArgs, "--locale-provider=icu") @@ -1162,12 +1164,13 @@ func TestResolveShard_PVCDeletionPolicy(t *testing.T) { ShardTemplateCache: make(map[string]*multigresv1alpha1.ShardTemplate), } - _, _, policy, _, _, _, err := r.ResolveShard(t.Context(), &multigresv1alpha1.ShardConfig{ + resolved, err := r.ResolveShard(t.Context(), &multigresv1alpha1.ShardConfig{ ShardTemplate: "tpl-pvc", }, ResolveShardOptions{}) if err != nil { t.Fatalf("unexpected error: %v", err) } + policy := resolved.PVCDeletionPolicy if policy == nil || policy.WhenDeleted != multigresv1alpha1.DeletePVCRetentionPolicy { t.Errorf("Expected Template PVCDeletionPolicy=Delete, got %v", policy) } @@ -1180,7 +1183,7 @@ func TestResolveShard_PVCDeletionPolicy(t *testing.T) { ShardTemplateCache: make(map[string]*multigresv1alpha1.ShardTemplate), } - _, pools, _, _, _, _, err := r.ResolveShard(t.Context(), &multigresv1alpha1.ShardConfig{ + resolved, err := r.ResolveShard(t.Context(), &multigresv1alpha1.ShardConfig{ Spec: &multigresv1alpha1.ShardInlineSpec{ Pools: map[multigresv1alpha1.PoolName]multigresv1alpha1.PoolSpec{ "custom-pool": { @@ -1195,6 +1198,7 @@ func TestResolveShard_PVCDeletionPolicy(t *testing.T) { if err != nil { t.Fatalf("unexpected error: %v", err) } + pools := resolved.Pools if p, ok := pools["custom-pool"]; !ok { t.Fatal("Expected custom-pool to exist") } else { @@ -1313,7 +1317,7 @@ func TestResolveShard_InheritedBackup(t *testing.T) { }, } - _, _, _, backupCfg, _, _, err := r.ResolveShard( + resolved, err := r.ResolveShard( t.Context(), &multigresv1alpha1.ShardConfig{ Spec: &multigresv1alpha1.ShardInlineSpec{ @@ -1327,6 +1331,7 @@ func TestResolveShard_InheritedBackup(t *testing.T) { if err != nil { t.Fatalf("unexpected error: %v", err) } + backupCfg := resolved.Backup if backupCfg == nil { t.Fatal("backup config should not be nil") } @@ -1350,7 +1355,7 @@ func TestResolveShard_InheritedBackup(t *testing.T) { }, } - _, _, _, backupCfg, _, _, err := r.ResolveShard( + resolved, err := r.ResolveShard( t.Context(), &multigresv1alpha1.ShardConfig{ Spec: &multigresv1alpha1.ShardInlineSpec{ @@ -1370,6 +1375,7 @@ func TestResolveShard_InheritedBackup(t *testing.T) { if err != nil { t.Fatalf("unexpected error: %v", err) } + backupCfg := resolved.Backup if backupCfg.Filesystem.Path != "/shard-override" { t.Errorf("Path = %q, want /shard-override", backupCfg.Filesystem.Path) } @@ -1380,7 +1386,7 @@ func TestResolveShard_InheritedBackup(t *testing.T) { c := fake.NewClientBuilder().WithScheme(scheme).Build() r := NewResolver(c, "default") - _, _, _, backupCfg, _, _, err := r.ResolveShard( + resolved, err := r.ResolveShard( t.Context(), &multigresv1alpha1.ShardConfig{ Spec: &multigresv1alpha1.ShardInlineSpec{ @@ -1394,6 +1400,7 @@ func TestResolveShard_InheritedBackup(t *testing.T) { if err != nil { t.Fatalf("unexpected error: %v", err) } + backupCfg := resolved.Backup if backupCfg == nil { t.Fatal("backup config should not be nil (should get defaults)") } @@ -1418,14 +1425,14 @@ func TestMergeShardConfig_PostgresConfigRef(t *testing.T) { t.Run("template sets postgresConfigRef", func(t *testing.T) { t.Parallel() - _, _, _, _, _, ref := mergeShardConfig( + ref := mergeShardConfig( &multigresv1alpha1.ShardTemplate{ Spec: multigresv1alpha1.ShardTemplateSpec{ PostgresConfigRef: templateRef, }, }, nil, nil, nil, nil, - ) + ).PostgresConfigRef if ref == nil || ref.Name != "template-config" || ref.Key != "postgresql.conf" { t.Errorf("postgresConfigRef = %v, want %v", ref, templateRef) } @@ -1433,7 +1440,7 @@ func TestMergeShardConfig_PostgresConfigRef(t *testing.T) { t.Run("overrides replace template ref", func(t *testing.T) { t.Parallel() - _, _, _, _, _, ref := mergeShardConfig( + ref := mergeShardConfig( &multigresv1alpha1.ShardTemplate{ Spec: multigresv1alpha1.ShardTemplateSpec{ PostgresConfigRef: templateRef, @@ -1443,7 +1450,7 @@ func TestMergeShardConfig_PostgresConfigRef(t *testing.T) { PostgresConfigRef: overrideRef, }, nil, nil, nil, - ) + ).PostgresConfigRef if ref == nil || ref.Name != "override-config" || ref.Key != "custom.conf" { t.Errorf("postgresConfigRef = %v, want %v", ref, overrideRef) } @@ -1451,7 +1458,7 @@ func TestMergeShardConfig_PostgresConfigRef(t *testing.T) { t.Run("inline replaces template and overrides", func(t *testing.T) { t.Parallel() - _, _, _, _, _, ref := mergeShardConfig( + ref := mergeShardConfig( &multigresv1alpha1.ShardTemplate{ Spec: multigresv1alpha1.ShardTemplateSpec{ PostgresConfigRef: templateRef, @@ -1464,7 +1471,7 @@ func TestMergeShardConfig_PostgresConfigRef(t *testing.T) { PostgresConfigRef: inlineRef, }, nil, nil, - ) + ).PostgresConfigRef if ref == nil || ref.Name != "inline-config" || ref.Key != "inline.conf" { t.Errorf("postgresConfigRef = %v, want %v", ref, inlineRef) } @@ -1472,10 +1479,10 @@ func TestMergeShardConfig_PostgresConfigRef(t *testing.T) { t.Run("nil everywhere returns nil", func(t *testing.T) { t.Parallel() - _, _, _, _, _, ref := mergeShardConfig( + ref := mergeShardConfig( &multigresv1alpha1.ShardTemplate{}, nil, nil, nil, nil, - ) + ).PostgresConfigRef if ref != nil { t.Errorf("postgresConfigRef = %v, want nil", ref) } @@ -1483,13 +1490,13 @@ func TestMergeShardConfig_PostgresConfigRef(t *testing.T) { t.Run("only overrides set ref", func(t *testing.T) { t.Parallel() - _, _, _, _, _, ref := mergeShardConfig( + ref := mergeShardConfig( nil, &multigresv1alpha1.ShardOverrides{ PostgresConfigRef: overrideRef, }, nil, nil, nil, - ) + ).PostgresConfigRef if ref == nil || ref.Name != "override-config" { t.Errorf("postgresConfigRef = %v, want %v", ref, overrideRef) } @@ -1497,13 +1504,13 @@ func TestMergeShardConfig_PostgresConfigRef(t *testing.T) { t.Run("only inline sets ref", func(t *testing.T) { t.Parallel() - _, _, _, _, _, ref := mergeShardConfig( + ref := mergeShardConfig( nil, nil, &multigresv1alpha1.ShardInlineSpec{ PostgresConfigRef: inlineRef, }, nil, nil, - ) + ).PostgresConfigRef if ref == nil || ref.Name != "inline-config" { t.Errorf("postgresConfigRef = %v, want %v", ref, inlineRef) } @@ -1511,7 +1518,7 @@ func TestMergeShardConfig_PostgresConfigRef(t *testing.T) { t.Run("nil overrides do not clear template ref", func(t *testing.T) { t.Parallel() - _, _, _, _, _, ref := mergeShardConfig( + ref := mergeShardConfig( &multigresv1alpha1.ShardTemplate{ Spec: multigresv1alpha1.ShardTemplateSpec{ PostgresConfigRef: templateRef, @@ -1519,7 +1526,7 @@ func TestMergeShardConfig_PostgresConfigRef(t *testing.T) { }, &multigresv1alpha1.ShardOverrides{}, nil, nil, nil, - ) + ).PostgresConfigRef if ref == nil || ref.Name != "template-config" { t.Errorf( "postgresConfigRef = %v, want %v (nil override should not clear template)", @@ -1529,3 +1536,75 @@ func TestMergeShardConfig_PostgresConfigRef(t *testing.T) { } }) } + +func TestMergeShardConfig_PostgresConfig(t *testing.T) { + t.Parallel() + + t.Run("nil everywhere returns nil", func(t *testing.T) { + t.Parallel() + cfg := mergeShardConfig( + &multigresv1alpha1.ShardTemplate{}, + nil, nil, nil, nil, + ).PostgresConfig + if cfg != nil { + t.Errorf("postgresConfig = %v, want nil", cfg) + } + }) + + t.Run("layers merge per key with inline winning", func(t *testing.T) { + t.Parallel() + cfg := mergeShardConfig( + &multigresv1alpha1.ShardTemplate{ + Spec: multigresv1alpha1.ShardTemplateSpec{ + PostgresConfig: map[string]string{ + "max_connections": "100", + "shared_buffers": "2GB", + }, + }, + }, + &multigresv1alpha1.ShardOverrides{ + PostgresConfig: map[string]string{ + "max_connections": "150", + "work_mem": "8MB", + }, + }, + &multigresv1alpha1.ShardInlineSpec{ + PostgresConfig: map[string]string{ + "max_connections": "200", + }, + }, + nil, nil, + ).PostgresConfig + want := map[string]string{ + "max_connections": "200", // inline > override > template + "shared_buffers": "2GB", // only in template + "work_mem": "8MB", // only in override + } + if len(cfg) != len(want) { + t.Fatalf("postgresConfig = %v, want %v", cfg, want) + } + for k, v := range want { + if cfg[k] != v { + t.Errorf("postgresConfig[%q] = %q, want %q", k, cfg[k], v) + } + } + }) + + t.Run("does not mutate the template map", func(t *testing.T) { + t.Parallel() + tplMap := map[string]string{"max_connections": "100"} + tpl := &multigresv1alpha1.ShardTemplate{ + Spec: multigresv1alpha1.ShardTemplateSpec{PostgresConfig: tplMap}, + } + _ = mergeShardConfig( + tpl, + &multigresv1alpha1.ShardOverrides{ + PostgresConfig: map[string]string{"max_connections": "200"}, + }, + nil, nil, nil, + ) + if tplMap["max_connections"] != "100" { + t.Errorf("template map mutated: %v", tplMap) + } + }) +} diff --git a/pkg/resolver/validation.go b/pkg/resolver/validation.go index 9b334d4c..85e495ce 100644 --- a/pkg/resolver/validation.go +++ b/pkg/resolver/validation.go @@ -386,7 +386,7 @@ func (r *Resolver) ValidateClusterLogic( // ------------------------------------------------------------------ // Dry-Run Resolution // We pass allCellNames just like the Reconciler would, to simulate the final state - orch, pools, _, backupCfg, _, _, err := r.ResolveShard( + resolved, err := r.ResolveShard( ctx, &shard, ResolveShardOptions{ @@ -402,6 +402,7 @@ func (r *Resolver) ValidateClusterLogic( err, ) } + orch, pools, backupCfg := &resolved.Multiorch, resolved.Pools, resolved.Backup // Pool Name Format: CRD structural schema does not enforce // validation markers on map keys, so validate explicitly. @@ -599,7 +600,7 @@ func (r *Resolver) ValidateClusterLogic( for _, tg := range db.TableGroups { tgBackup := multigresv1alpha1.MergeBackupConfig(tg.Backup, dbBackup) for _, shard := range tg.Shards { - _, pools, _, backupCfg, _, _, err := r.ResolveShard( + resolved, err := r.ResolveShard( ctx, &shard, ResolveShardOptions{ @@ -611,6 +612,7 @@ func (r *Resolver) ValidateClusterLogic( if err != nil { continue // already validated in section 2 } + pools, backupCfg := resolved.Pools, resolved.Backup for poolName, pool := range pools { if pool.Storage.Class == "" { diff --git a/pkg/resource-handler/controller/shard/configmap.go b/pkg/resource-handler/controller/shard/configmap.go index 8b88d559..e8562391 100644 --- a/pkg/resource-handler/controller/shard/configmap.go +++ b/pkg/resource-handler/controller/shard/configmap.go @@ -49,3 +49,33 @@ func BuildPgHbaConfigMap( return cm, nil } + +// BuildPostgresConfigMap creates the operator-owned ConfigMap holding the +// rendered postgresql.conf for a shard. It is shared across all pools in the +// shard and mounted into every pool pod. +func BuildPostgresConfigMap( + shard *multigresv1alpha1.Shard, + rendered string, + scheme *runtime.Scheme, +) (*corev1.ConfigMap, error) { + clusterName := shard.Labels["multigres.com/cluster"] + labels := metadata.BuildStandardLabels(clusterName, "postgres-config") + labels = metadata.MergeLabels(labels, shard.GetObjectMeta().GetLabels()) + + cm := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: PostgresConfigMapName(shard.Name), + Namespace: shard.Namespace, + Labels: labels, + }, + Data: map[string]string{ + PostgresConfigMapKey: rendered, + }, + } + + if err := ctrl.SetControllerReference(shard, cm, scheme); err != nil { + return nil, fmt.Errorf("failed to set controller reference: %w", err) + } + + return cm, nil +} diff --git a/pkg/resource-handler/controller/shard/configmap_test.go b/pkg/resource-handler/controller/shard/configmap_test.go index 790749e6..090d0061 100644 --- a/pkg/resource-handler/controller/shard/configmap_test.go +++ b/pkg/resource-handler/controller/shard/configmap_test.go @@ -118,6 +118,51 @@ func TestBuildPgHbaConfigMap(t *testing.T) { } } +func TestBuildPostgresConfigMap(t *testing.T) { + scheme := runtime.NewScheme() + _ = multigresv1alpha1.AddToScheme(scheme) + + shard := &multigresv1alpha1.Shard{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-shard", + Namespace: "default", + UID: "test-uid", + Labels: map[string]string{"multigres.com/cluster": "test-cluster"}, + }, + } + + t.Run("stores rendered content under the config key with an owner ref", func(t *testing.T) { + rendered := "# rendered\nmax_connections = 200\n" + cm, err := BuildPostgresConfigMap(shard, rendered, scheme) + if err != nil { + t.Fatalf("BuildPostgresConfigMap() error = %v", err) + } + if cm.Name != PostgresConfigMapName(shard.Name) { + t.Errorf("name = %q, want %q", cm.Name, PostgresConfigMapName(shard.Name)) + } + if cm.Namespace != shard.Namespace { + t.Errorf("namespace = %q, want %q", cm.Namespace, shard.Namespace) + } + if got := cm.Data[PostgresConfigMapKey]; got != rendered { + t.Errorf("Data[%q] = %q, want %q", PostgresConfigMapKey, got, rendered) + } + if len(cm.OwnerReferences) != 1 || + cm.OwnerReferences[0].Name != shard.Name || + cm.OwnerReferences[0].Kind != "Shard" { + t.Errorf("owner reference = %+v, want Shard/%s", cm.OwnerReferences, shard.Name) + } + if !ptr.Deref(cm.OwnerReferences[0].Controller, false) { + t.Error("expected owner reference to be controller") + } + }) + + t.Run("returns error on invalid scheme", func(t *testing.T) { + if _, err := BuildPostgresConfigMap(shard, "x", runtime.NewScheme()); err == nil { + t.Error("expected error with empty scheme") + } + }) +} + func TestDefaultPgHbaTemplateEmbedded(t *testing.T) { // Verify the embedded template is not empty if DefaultPgHbaTemplate == "" { diff --git a/pkg/resource-handler/controller/shard/containers.go b/pkg/resource-handler/controller/shard/containers.go index 6eb24878..f0f9b216 100644 --- a/pkg/resource-handler/controller/shard/containers.go +++ b/pkg/resource-handler/controller/shard/containers.go @@ -8,6 +8,7 @@ import ( "k8s.io/utils/ptr" multigresv1alpha1 "github.com/multigres/multigres-operator/api/v1alpha1" + "github.com/multigres/multigres-operator/pkg/postgresconfig" "github.com/multigres/multigres-operator/pkg/util/metadata" nameutil "github.com/multigres/multigres-operator/pkg/util/name" ) @@ -63,6 +64,10 @@ const ( // passed to pgctld via POSTGRES_INITDB_EXTRA_CONF. PostgresConfigFilePath = PostgresConfigMountPath + "/postgresql.conf" + // PostgresConfigMapKey is the key under which the operator stores the + // rendered postgresql.conf in its per-shard config ConfigMap. + PostgresConfigMapKey = postgresconfig.ConfigFileName + // PostgresPasswordSecretKey is the key within the Secret that holds the password PostgresPasswordSecretKey = "password" @@ -128,6 +133,14 @@ func PgHbaConfigMapName(shardName string) string { return shardName + "-pg-hba" } +// PostgresConfigMapName returns the per-shard ConfigMap name for the +// operator-rendered postgresql.conf. Config is rendered once per shard and +// mounted into every pod of the shard (see the design doc for why it is +// shard-level and not per-pool). +func PostgresConfigMapName(shardName string) string { + return shardName + "-postgres-config" +} + func postgresPasswordSecretRef(shard *multigresv1alpha1.Shard) (name, key string) { return shard.Spec.PostgresPasswordSecretRef.Name, shard.Spec.PostgresPasswordSecretRef.Key } @@ -272,12 +285,12 @@ func buildPgctldSidecar( Value: string(shard.Spec.InitdbArgs), }) } - if shard.Spec.PostgresConfigRef != nil { - env = append(env, corev1.EnvVar{ - Name: "POSTGRES_INITDB_EXTRA_CONF", - Value: PostgresConfigFilePath, - }) - } + // The operator always renders postgresql.conf (baseline + overrides), so + // pgctld always reads the operator-owned config file. + env = append(env, corev1.EnvVar{ + Name: "POSTGRES_INITDB_EXTRA_CONF", + Value: PostgresConfigFilePath, + }) env = append(env, s3EnvVars(shard.Spec.Backup)...) if otelVars := buildRuntimeOTELEnvVars(shard, "pgctld"); len(otelVars) > 0 { env = append(env, otelVars...) @@ -302,13 +315,11 @@ func buildPgctldSidecar( ReadOnly: true, }, postgresPasswordVolumeMount(), - } - if shard.Spec.PostgresConfigRef != nil { - volumeMounts = append(volumeMounts, corev1.VolumeMount{ + { Name: PostgresConfigVolumeName, MountPath: PostgresConfigMountPath, ReadOnly: true, - }) + }, } if shard.Spec.Backup != nil { volumeMounts = append(volumeMounts, corev1.VolumeMount{ @@ -671,19 +682,19 @@ func buildRuntimeOTELEnvVars( // buildPoolVolumes assembles the complete list of volumes for a pool pod. // Conditionally includes the pgBackRest cert volume when backup is configured. -// buildPostgresConfigVolume creates a volume that projects a specific key from -// the user-provided ConfigMap to the expected postgresql.conf filename so -// pgctld picks it up via POSTGRES_INITDB_EXTRA_CONF. -func buildPostgresConfigVolume(ref *multigresv1alpha1.PostgresConfigRef) corev1.Volume { +// buildPostgresConfigVolume projects the operator-rendered postgresql.conf from +// the per-shard config ConfigMap to the expected filename so pgctld picks it up +// via POSTGRES_INITDB_EXTRA_CONF. +func buildPostgresConfigVolume(shard *multigresv1alpha1.Shard) corev1.Volume { return corev1.Volume{ Name: PostgresConfigVolumeName, VolumeSource: corev1.VolumeSource{ ConfigMap: &corev1.ConfigMapVolumeSource{ LocalObjectReference: corev1.LocalObjectReference{ - Name: ref.Name, + Name: PostgresConfigMapName(shard.Name), }, Items: []corev1.KeyToPath{ - {Key: ref.Key, Path: "postgresql.conf"}, + {Key: PostgresConfigMapKey, Path: "postgresql.conf"}, }, }, }, @@ -696,9 +707,7 @@ func buildPoolVolumes(shard *multigresv1alpha1.Shard, cellName string) []corev1. buildSocketDirVolume(), buildPgHbaVolume(shard.Name), buildPostgresPasswordVolume(shard), - } - if shard.Spec.PostgresConfigRef != nil { - volumes = append(volumes, buildPostgresConfigVolume(shard.Spec.PostgresConfigRef)) + buildPostgresConfigVolume(shard), } if certVol := buildPgBackRestCertVolume(shard); certVol != nil { volumes = append(volumes, *certVol) diff --git a/pkg/resource-handler/controller/shard/containers_test.go b/pkg/resource-handler/controller/shard/containers_test.go index d3f9dc31..49688a65 100644 --- a/pkg/resource-handler/controller/shard/containers_test.go +++ b/pkg/resource-handler/controller/shard/containers_test.go @@ -830,68 +830,57 @@ func TestBuildPgctldSidecar(t *testing.T) { assertNotContainsEnvVar(t, c.Env, "POSTGRES_INITDB_ARGS") }) - t.Run("has POSTGRES_INITDB_EXTRA_CONF env when postgresConfigRef set", func(t *testing.T) { - shard := &multigresv1alpha1.Shard{Spec: multigresv1alpha1.ShardSpec{ - PostgresConfigRef: &multigresv1alpha1.PostgresConfigRef{ - Name: "my-pg-config", - Key: "custom.conf", - }, - }} - c := buildPgctldSidecar(shard, multigresv1alpha1.PoolSpec{}) - assertContainsEnvVar(t, c.Env, "POSTGRES_INITDB_EXTRA_CONF") - for _, e := range c.Env { - if e.Name == "POSTGRES_INITDB_EXTRA_CONF" { - if e.Value != PostgresConfigFilePath { - t.Errorf( - "POSTGRES_INITDB_EXTRA_CONF = %q, want %q", - e.Value, - PostgresConfigFilePath, - ) + // The operator always renders postgresql.conf, so pgctld always gets the + // POSTGRES_INITDB_EXTRA_CONF env and the config volume mount — with or + // without a legacy PostgresConfigRef. + t.Run("always sets POSTGRES_INITDB_EXTRA_CONF env", func(t *testing.T) { + for name, shard := range map[string]*multigresv1alpha1.Shard{ + "with ref": {Spec: multigresv1alpha1.ShardSpec{ + PostgresConfigRef: &multigresv1alpha1.PostgresConfigRef{Name: "my-pg-config", Key: "custom.conf"}, + }}, + "without ref": {Spec: multigresv1alpha1.ShardSpec{}}, + } { + t.Run(name, func(t *testing.T) { + c := buildPgctldSidecar(shard, multigresv1alpha1.PoolSpec{}) + assertContainsEnvVar(t, c.Env, "POSTGRES_INITDB_EXTRA_CONF") + for _, e := range c.Env { + if e.Name == "POSTGRES_INITDB_EXTRA_CONF" && e.Value != PostgresConfigFilePath { + t.Errorf( + "POSTGRES_INITDB_EXTRA_CONF = %q, want %q", + e.Value, + PostgresConfigFilePath, + ) + } } - } + }) } }) - t.Run("no POSTGRES_INITDB_EXTRA_CONF env when postgresConfigRef nil", func(t *testing.T) { - shard := &multigresv1alpha1.Shard{Spec: multigresv1alpha1.ShardSpec{}} - c := buildPgctldSidecar(shard, multigresv1alpha1.PoolSpec{}) - assertNotContainsEnvVar(t, c.Env, "POSTGRES_INITDB_EXTRA_CONF") - }) - - t.Run("has postgres config volume mount when postgresConfigRef set", func(t *testing.T) { - shard := &multigresv1alpha1.Shard{Spec: multigresv1alpha1.ShardSpec{ - PostgresConfigRef: &multigresv1alpha1.PostgresConfigRef{ - Name: "my-pg-config", - Key: "custom.conf", - }, - }} - c := buildPgctldSidecar(shard, multigresv1alpha1.PoolSpec{}) - assertContainsVolumeMount(t, c.VolumeMounts, PostgresConfigVolumeName) - for _, m := range c.VolumeMounts { - if m.Name == PostgresConfigVolumeName { - if m.MountPath != PostgresConfigMountPath { - t.Errorf( - "postgres config mount path = %q, want %q", - m.MountPath, - PostgresConfigMountPath, - ) - } - if !m.ReadOnly { - t.Error("postgres config volume mount should be read-only") + t.Run("always mounts the postgres config volume read-only", func(t *testing.T) { + for name, shard := range map[string]*multigresv1alpha1.Shard{ + "with ref": {Spec: multigresv1alpha1.ShardSpec{ + PostgresConfigRef: &multigresv1alpha1.PostgresConfigRef{Name: "my-pg-config", Key: "custom.conf"}, + }}, + "without ref": {Spec: multigresv1alpha1.ShardSpec{}}, + } { + t.Run(name, func(t *testing.T) { + c := buildPgctldSidecar(shard, multigresv1alpha1.PoolSpec{}) + assertContainsVolumeMount(t, c.VolumeMounts, PostgresConfigVolumeName) + for _, m := range c.VolumeMounts { + if m.Name == PostgresConfigVolumeName { + if m.MountPath != PostgresConfigMountPath { + t.Errorf( + "postgres config mount path = %q, want %q", + m.MountPath, + PostgresConfigMountPath, + ) + } + if !m.ReadOnly { + t.Error("postgres config volume mount should be read-only") + } + } } - } - } - }) - - t.Run("no postgres config volume mount when postgresConfigRef nil", func(t *testing.T) { - shard := &multigresv1alpha1.Shard{Spec: multigresv1alpha1.ShardSpec{}} - c := buildPgctldSidecar(shard, multigresv1alpha1.PoolSpec{}) - for _, m := range c.VolumeMounts { - if m.Name == PostgresConfigVolumeName { - t.Error( - "should not have postgres config volume mount when postgresConfigRef is nil", - ) - } + }) } }) } @@ -1790,18 +1779,15 @@ func TestBuildPoolVolumes_CertVolumePresence(t *testing.T) { } }) - t.Run("postgres config volume present when postgresConfigRef set", func(t *testing.T) { + t.Run("always projects the operator-owned postgres config ConfigMap", func(t *testing.T) { shard := &multigresv1alpha1.Shard{ ObjectMeta: metav1.ObjectMeta{ Name: "test-shard", Labels: map[string]string{"multigres.com/cluster": "test"}, }, - Spec: multigresv1alpha1.ShardSpec{ - PostgresConfigRef: &multigresv1alpha1.PostgresConfigRef{ - Name: "my-pg-config", - Key: "custom.conf", - }, - }, + // No PostgresConfigRef: the operator still renders and mounts its own + // ConfigMap. + Spec: multigresv1alpha1.ShardSpec{}, } volumes := buildPoolVolumes(shard, "zone1") found := false @@ -1809,19 +1795,20 @@ func TestBuildPoolVolumes_CertVolumePresence(t *testing.T) { if v.Name == PostgresConfigVolumeName { found = true if v.ConfigMap == nil { - t.Error("postgres config volume should use ConfigMap source") - } else { - if v.ConfigMap.Name != "my-pg-config" { - t.Errorf("postgres config ConfigMap name = %q, want %q", - v.ConfigMap.Name, "my-pg-config") - } - if len(v.ConfigMap.Items) != 1 || v.ConfigMap.Items[0].Key != "custom.conf" || - v.ConfigMap.Items[0].Path != "postgresql.conf" { - t.Errorf( - "postgres config ConfigMap items = %+v, want [{Key:custom.conf Path:postgresql.conf}]", - v.ConfigMap.Items, - ) - } + t.Fatal("postgres config volume should use ConfigMap source") + } + if v.ConfigMap.Name != PostgresConfigMapName("test-shard") { + t.Errorf("postgres config ConfigMap name = %q, want %q", + v.ConfigMap.Name, PostgresConfigMapName("test-shard")) + } + if len(v.ConfigMap.Items) != 1 || + v.ConfigMap.Items[0].Key != PostgresConfigMapKey || + v.ConfigMap.Items[0].Path != "postgresql.conf" { + t.Errorf( + "postgres config ConfigMap items = %+v, want [{Key:%s Path:postgresql.conf}]", + v.ConfigMap.Items, + PostgresConfigMapKey, + ) } break } @@ -1831,22 +1818,6 @@ func TestBuildPoolVolumes_CertVolumePresence(t *testing.T) { } }) - t.Run("no postgres config volume when postgresConfigRef nil", func(t *testing.T) { - shard := &multigresv1alpha1.Shard{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-shard", - Labels: map[string]string{"multigres.com/cluster": "test"}, - }, - Spec: multigresv1alpha1.ShardSpec{}, - } - volumes := buildPoolVolumes(shard, "zone1") - for _, v := range volumes { - if v.Name == PostgresConfigVolumeName { - t.Error("should not have postgres config volume when postgresConfigRef is nil") - } - } - }) - t.Run("internal multipooler tls volume is present when enabled", func(t *testing.T) { shard := &multigresv1alpha1.Shard{ ObjectMeta: metav1.ObjectMeta{ diff --git a/pkg/resource-handler/controller/shard/export_test.go b/pkg/resource-handler/controller/shard/export_test.go new file mode 100644 index 00000000..148a4441 --- /dev/null +++ b/pkg/resource-handler/controller/shard/export_test.go @@ -0,0 +1,7 @@ +package shard + +// RenderPostgresConfig re-exports the unexported renderPostgresConfig to +// black-box (shard_test) tests so they can reproduce the rendered-config hash +// the controller stamps on pods. Production code uses renderEffectiveConfig / +// renderPostgresConfig directly; this handle exists only in the test binary. +var RenderPostgresConfig = renderPostgresConfig diff --git a/pkg/resource-handler/controller/shard/integration_test.go b/pkg/resource-handler/controller/shard/integration_test.go index 5150e3a0..72eb5e03 100644 --- a/pkg/resource-handler/controller/shard/integration_test.go +++ b/pkg/resource-handler/controller/shard/integration_test.go @@ -938,6 +938,19 @@ func TestShardReconciliation(t *testing.T) { filteredResources := append([]client.Object{}, tc.wantResources...) + // The controller stamps the rendered-config hash on the shard, which + // BuildPoolPod folds into each pod's spec-hash. Reproduce it here so + // the expected pods match the ones the controller creates. These test + // shards have no PostgresConfigRef, so the ref content is empty. + _, configHash, err := shardcontroller.RenderPostgresConfig(tc.shard, "") + if err != nil { + t.Fatalf("Failed to render postgres config hash: %v", err) + } + if tc.shard.Annotations == nil { + tc.shard.Annotations = map[string]string{} + } + tc.shard.Annotations[metadata.AnnotationPostgresConfigHash] = configHash + // Append literal expected Pods and PVCs based on Shard Spec backupCells := map[string]bool{} for poolName, poolSpec := range tc.shard.Spec.Pools { diff --git a/pkg/resource-handler/controller/shard/postgres_config.go b/pkg/resource-handler/controller/shard/postgres_config.go new file mode 100644 index 00000000..a1871deb --- /dev/null +++ b/pkg/resource-handler/controller/shard/postgres_config.go @@ -0,0 +1,259 @@ +package shard + +import ( + "context" + "crypto/sha256" + "encoding/hex" + "fmt" + "strings" + + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + multigresv1alpha1 "github.com/multigres/multigres-operator/api/v1alpha1" + "github.com/multigres/multigres-operator/pkg/postgresconfig" + "github.com/multigres/multigres-operator/pkg/util/metadata" +) + +// renderedConfig bundles the once-per-reconcile render result so the status path +// (drift detection via the hash) and the ConfigMap-delivery path +// (reconcilePostgresConfig) share a single value instead of threading the +// content, hash, and error positionally through every signature. +type renderedConfig struct { + content string + hash string + err error +} + +// renderEffectiveConfig renders the shard's effective postgresql.conf once per +// reconcile. Both the status path (drift detection via the hash) and +// reconcilePostgresConfig (ConfigMap delivery) consume this single result, so the +// legacy PostgresConfigRef ConfigMap is fetched and the template rendered exactly +// once. +func (r *ShardReconciler) renderEffectiveConfig( + ctx context.Context, + shard *multigresv1alpha1.Shard, +) renderedConfig { + var refContent string + if shard.Spec.PostgresConfigRef != nil { + content, cerr := r.postgresConfigRefContent(ctx, shard) + if cerr != nil { + return renderedConfig{err: cerr} + } + refContent = content + } + rendered, hash, err := renderPostgresConfig(shard, refContent) + return renderedConfig{content: rendered, hash: hash, err: err} +} + +// reconcilePostgresConfig delivers the shard's effective postgresql.conf into +// the operator-owned ConfigMap and stamps its content hash on the shard as an +// in-memory annotation, so that config changes produce a different pod spec-hash +// and trigger the existing rolling update mechanism. cfg comes from +// renderEffectiveConfig (run once per reconcile); a render/read failure fails the +// reconcile here, after updateStatus has already surfaced it in the config +// status. Delivery always happens, so every shard gets an operator-owned config +// ConfigMap. +func (r *ShardReconciler) reconcilePostgresConfig( + ctx context.Context, + shard *multigresv1alpha1.Shard, + cfg renderedConfig, +) error { + if cfg.err != nil { + return cfg.err + } + + if err := r.applyPostgresConfigMap(ctx, shard, cfg.content); err != nil { + return err + } + + if shard.Annotations == nil { + shard.Annotations = make(map[string]string) + } + shard.Annotations[metadata.AnnotationPostgresConfigHash] = cfg.hash + return nil +} + +// renderPostgresConfig renders the effective postgresql.conf for a shard and +// returns it together with the content hash stamped on +// AnnotationPostgresConfigHash. It applies resource-derived sizing computed from +// the shard's pool resources (see reduceShardResources), and layers the shard's +// inline spec.postgresConfig map on top. refContent is the legacy +// PostgresConfigRef body (empty when unset). It is re-exported to black-box tests +// as RenderPostgresConfig via export_test.go so they can reproduce the hash the +// controller stamps. +func renderPostgresConfig( + shard *multigresv1alpha1.Shard, + refContent string, +) (rendered, hash string, err error) { + cfg := postgresconfig.Defaults() + cfg.ClusterName = shardClusterName(shard) + memBytes, cpuMillicores, diskBytes := reduceShardResources(shard) + if err := postgresconfig.ApplyResourceSizing( + &cfg, + memBytes, + cpuMillicores, + diskBytes, + ); err != nil { + return "", "", err + } + + rendered, err = postgresconfig.Render(cfg, refContent, shard.Spec.PostgresConfig) + if err != nil { + return "", "", err + } + sum := sha256.Sum256([]byte(rendered)) + return rendered, hex.EncodeToString(sum[:]), nil +} + +// shardClusterName builds the postgresql.conf cluster_name for a shard: a +// slash-joined cluster/database/tablegroup/shard path so the owning shard is +// identifiable in Postgres process titles and logs (every shard is a distinct +// set of Postgres instances). Empty components are dropped, and it falls back to +// the Shard object name when the cluster label is absent (e.g. a Shard applied +// directly without the operator's labels). The components are DNS-safe k8s names +// and validated identifiers, so they never contain the single quote the template +// wraps this value in. +func shardClusterName(shard *multigresv1alpha1.Shard) string { + parts := make([]string, 0, 4) + for _, p := range []string{ + shard.Labels[metadata.LabelMultigresCluster], + string(shard.Spec.DatabaseName), + string(shard.Spec.TableGroupName), + string(shard.Spec.ShardName), + } { + if p != "" { + parts = append(parts, p) + } + } + if len(parts) == 0 { + return shard.Name + } + return strings.Join(parts, "/") +} + +// reduceShardResources reduces the shard's per-pool resources to the single +// basis used for config sizing: memory and CPU take the max across pools (so the +// replication-sensitive settings stay valid on every pod), disk takes the min +// (so WAL budgeting never overfills the smallest volume). Each value falls back +// from limit to request. Absent inputs return zero, which leaves the baseline. +func reduceShardResources( + shard *multigresv1alpha1.Shard, +) (memBytes, cpuMillicores, diskBytes int64) { + for _, pool := range shard.Spec.Pools { + memQ := effectiveResource(pool.Postgres.Resources, corev1.ResourceMemory) + if mem := memQ.Value(); mem > memBytes { + memBytes = mem + } + cpuQ := effectiveResource(pool.Postgres.Resources, corev1.ResourceCPU) + if cpu := cpuQ.MilliValue(); cpu > cpuMillicores { + cpuMillicores = cpu + } + if disk := parseStorageBytes( + pool.Storage.Size, + ); disk > 0 && + (diskBytes == 0 || disk < diskBytes) { + diskBytes = disk + } + } + return memBytes, cpuMillicores, diskBytes +} + +// effectiveResource returns the limit for a resource, falling back to the +// request, matching "size off the ceiling, guaranteed floor otherwise". +func effectiveResource( + res corev1.ResourceRequirements, + name corev1.ResourceName, +) resource.Quantity { + if q, ok := res.Limits[name]; ok && !q.IsZero() { + return q + } + if q, ok := res.Requests[name]; ok && !q.IsZero() { + return q + } + return resource.Quantity{} +} + +func parseStorageBytes(size string) int64 { + if size == "" { + return 0 + } + q, err := resource.ParseQuantity(size) + if err != nil { + return 0 + } + return q.Value() +} + +// postgresConfigRefContent fetches the body of the shard's legacy +// PostgresConfigRef ConfigMap key. The caller guarantees PostgresConfigRef is +// non-nil. +func (r *ShardReconciler) postgresConfigRefContent( + ctx context.Context, + shard *multigresv1alpha1.Shard, +) (string, error) { + ref := shard.Spec.PostgresConfigRef + + cm := &corev1.ConfigMap{} + if err := r.Get(ctx, client.ObjectKey{ + Namespace: shard.Namespace, + Name: ref.Name, + }, cm); err != nil { + return "", fmt.Errorf("failed to get ConfigMap %q: %w", ref.Name, err) + } + + data, ok := cm.Data[ref.Key] + if !ok { + return "", fmt.Errorf("key %q not found in ConfigMap %q", ref.Key, ref.Name) + } + return data, nil +} + +// applyPostgresConfigMap server-side-applies the operator-owned ConfigMap that +// holds the rendered postgresql.conf for a shard. +func (r *ShardReconciler) applyPostgresConfigMap( + ctx context.Context, + shard *multigresv1alpha1.Shard, + rendered string, +) error { + desired, err := BuildPostgresConfigMap(shard, rendered, r.Scheme) + if err != nil { + return fmt.Errorf("failed to build postgres config ConfigMap: %w", err) + } + + desired.SetGroupVersionKind(corev1.SchemeGroupVersion.WithKind("ConfigMap")) + if err := r.Patch( + ctx, + desired, + client.Apply, + client.ForceOwnership, + client.FieldOwner("multigres-operator"), + ); err != nil { + return fmt.Errorf("failed to apply postgres config ConfigMap: %w", err) + } + return nil +} + +// enqueueFromPostgresConfigMap returns reconcile requests for Shards that +// reference the changed ConfigMap via spec.postgresConfigRef.name. +func (r *ShardReconciler) enqueueFromPostgresConfigMap( + ctx context.Context, + o client.Object, +) []reconcile.Request { + shards := &multigresv1alpha1.ShardList{} + if err := r.List(ctx, shards, client.InNamespace(o.GetNamespace())); err != nil { + return nil + } + + var requests []reconcile.Request + for _, s := range shards.Items { + if s.Spec.PostgresConfigRef != nil && s.Spec.PostgresConfigRef.Name == o.GetName() { + requests = append(requests, reconcile.Request{ + NamespacedName: client.ObjectKeyFromObject(&s), + }) + } + } + return requests +} diff --git a/pkg/resource-handler/controller/shard/postgres_config_test.go b/pkg/resource-handler/controller/shard/postgres_config_test.go new file mode 100644 index 00000000..55b5c343 --- /dev/null +++ b/pkg/resource-handler/controller/shard/postgres_config_test.go @@ -0,0 +1,414 @@ +package shard + +import ( + "context" + "strings" + "testing" + "time" + + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + multigresv1alpha1 "github.com/multigres/multigres-operator/api/v1alpha1" + "github.com/multigres/multigres-operator/pkg/util/metadata" +) + +func TestSetPostgresConfigStatus(t *testing.T) { + r := &ShardReconciler{} + + t.Run("settled clears InProgress and stamps time", func(t *testing.T) { + shard := &multigresv1alpha1.Shard{} + r.setPostgresConfigStatus(shard, false, nil) + st := shard.Status.PostgresConfig + if st == nil || st.InProgress { + t.Fatalf("status = %+v, want InProgress false", st) + } + if st.LastAppliedAt == nil { + t.Error("LastAppliedAt should be set when settled") + } + if st.Error != "" { + t.Errorf("Error = %q, want empty", st.Error) + } + }) + + t.Run("in-progress sets InProgress and does not stamp", func(t *testing.T) { + shard := &multigresv1alpha1.Shard{} + r.setPostgresConfigStatus(shard, true, nil) + st := shard.Status.PostgresConfig + if !st.InProgress { + t.Error("InProgress should be true during a rollout") + } + if st.LastAppliedAt != nil { + t.Error("LastAppliedAt should not be stamped while a rollout is in progress") + } + }) + + // The key fix: a rollout driven by a PostgresConfigRef edit (or a new + // operator baseline) never bumps the shard generation, yet InProgress must + // still report it — the signal is content-based, not generation-based. + t.Run("reports a rollout that does not bump generation", func(t *testing.T) { + past := metav1.NewTime(time.Now().Add(-time.Hour)) + shard := &multigresv1alpha1.Shard{ + ObjectMeta: metav1.ObjectMeta{Generation: 5}, + Status: multigresv1alpha1.ShardStatus{ + PostgresConfig: &multigresv1alpha1.PostgresConfigStatus{LastAppliedAt: &past}, + }, + } + r.setPostgresConfigStatus(shard, true, nil) + if !shard.Status.PostgresConfig.InProgress { + t.Error("InProgress should be true for a content-driven rollout at a steady generation") + } + }) + + t.Run("settling after a rollout re-stamps LastAppliedAt", func(t *testing.T) { + past := metav1.NewTime(time.Now().Add(-time.Hour)) + shard := &multigresv1alpha1.Shard{ + Status: multigresv1alpha1.ShardStatus{ + PostgresConfig: &multigresv1alpha1.PostgresConfigStatus{ + InProgress: true, + LastAppliedAt: &past, + }, + }, + } + r.setPostgresConfigStatus(shard, false, nil) + st := shard.Status.PostgresConfig + if st.InProgress { + t.Error("InProgress should clear once the config settles") + } + if st.LastAppliedAt == nil || !st.LastAppliedAt.After(past.Time) { + t.Errorf("LastAppliedAt should advance on settle, got %v", st.LastAppliedAt) + } + }) + + t.Run("steady-state settled does not churn LastAppliedAt", func(t *testing.T) { + past := metav1.NewTime(time.Now().Add(-time.Hour)) + shard := &multigresv1alpha1.Shard{ + Status: multigresv1alpha1.ShardStatus{ + PostgresConfig: &multigresv1alpha1.PostgresConfigStatus{LastAppliedAt: &past}, + }, + } + r.setPostgresConfigStatus(shard, false, nil) + if got := shard.Status.PostgresConfig.LastAppliedAt; got == nil || !got.Equal(&past) { + t.Errorf("LastAppliedAt should stay stable when already settled, got %v", got) + } + }) + + t.Run("config error is reported and is not in progress", func(t *testing.T) { + shard := &multigresv1alpha1.Shard{} + r.setPostgresConfigStatus(shard, false, errTest) + st := shard.Status.PostgresConfig + if st.InProgress { + t.Error("InProgress should be false when a config error is reported") + } + if !strings.Contains(st.Error, "boom") { + t.Errorf("Error = %q, want it to contain the failure", st.Error) + } + }) +} + +func TestRenderEffectiveConfig(t *testing.T) { + scheme := runtime.NewScheme() + _ = multigresv1alpha1.AddToScheme(scheme) + _ = corev1.AddToScheme(scheme) + + t.Run("no ref returns a stable hash", func(t *testing.T) { + r := &ShardReconciler{Client: fake.NewClientBuilder().WithScheme(scheme).Build()} + shard := &multigresv1alpha1.Shard{ObjectMeta: metav1.ObjectMeta{Name: "s1"}} + rc := r.renderEffectiveConfig(context.Background(), shard) + if rc.err != nil { + t.Fatalf("unexpected error: %v", rc.err) + } + if len(rc.hash) != 64 { + t.Errorf("hash length = %d, want 64", len(rc.hash)) + } + }) + + t.Run("missing ref ConfigMap surfaces an error", func(t *testing.T) { + r := &ShardReconciler{Client: fake.NewClientBuilder().WithScheme(scheme).Build()} + shard := &multigresv1alpha1.Shard{ + ObjectMeta: metav1.ObjectMeta{Name: "s1", Namespace: "default"}, + Spec: multigresv1alpha1.ShardSpec{ + PostgresConfigRef: &multigresv1alpha1.PostgresConfigRef{Name: "missing", Key: "k"}, + }, + } + if r.renderEffectiveConfig(context.Background(), shard).err == nil { + t.Error("expected error for missing ConfigMap") + } + }) +} + +var errTest = errTestType("boom") + +type errTestType string + +func (e errTestType) Error() string { return string(e) } + +func TestShardClusterName(t *testing.T) { + t.Run("joins cluster/db/tablegroup/shard", func(t *testing.T) { + shard := &multigresv1alpha1.Shard{ + ObjectMeta: metav1.ObjectMeta{ + Name: "obj-name", + Labels: map[string]string{metadata.LabelMultigresCluster: "mycluster"}, + }, + Spec: multigresv1alpha1.ShardSpec{ + DatabaseName: "mydb", + TableGroupName: "mytg", + ShardName: "0", + }, + } + if got := shardClusterName(shard); got != "mycluster/mydb/mytg/0" { + t.Errorf("shardClusterName = %q, want mycluster/mydb/mytg/0", got) + } + }) + + t.Run("drops empty components", func(t *testing.T) { + shard := &multigresv1alpha1.Shard{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{metadata.LabelMultigresCluster: "c"}, + }, + Spec: multigresv1alpha1.ShardSpec{ShardName: "0"}, + } + if got := shardClusterName(shard); got != "c/0" { + t.Errorf("shardClusterName = %q, want c/0", got) + } + }) + + t.Run("falls back to object name without cluster label", func(t *testing.T) { + shard := &multigresv1alpha1.Shard{ObjectMeta: metav1.ObjectMeta{Name: "fallback-shard"}} + if got := shardClusterName(shard); got != "fallback-shard" { + t.Errorf("shardClusterName = %q, want fallback-shard", got) + } + }) + + t.Run("rendered config carries the shard cluster_name", func(t *testing.T) { + shard := &multigresv1alpha1.Shard{ + ObjectMeta: metav1.ObjectMeta{ + Name: "obj-name", + Labels: map[string]string{metadata.LabelMultigresCluster: "mycluster"}, + }, + Spec: multigresv1alpha1.ShardSpec{ + DatabaseName: "mydb", + TableGroupName: "mytg", + ShardName: "0", + }, + } + rendered, _, err := renderPostgresConfig(shard, "") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !strings.Contains(rendered, "cluster_name = 'mycluster/mydb/mytg/0'") { + t.Errorf("rendered config missing shard cluster_name:\n%s", rendered) + } + }) +} + +func TestReduceShardResources(t *testing.T) { + t.Run( + "max mem/cpu, min disk across pools with limit-then-request fallback", + func(t *testing.T) { + shard := &multigresv1alpha1.Shard{ + Spec: multigresv1alpha1.ShardSpec{ + Pools: map[multigresv1alpha1.PoolName]multigresv1alpha1.PoolSpec{ + "a": { + // Limits present → used over requests. + Postgres: multigresv1alpha1.ContainerConfig{ + Resources: corev1.ResourceRequirements{ + Limits: corev1.ResourceList{ + corev1.ResourceMemory: resource.MustParse("1Gi"), + corev1.ResourceCPU: resource.MustParse("2"), + }, + }, + }, + Storage: multigresv1alpha1.StorageSpec{Size: "10Gi"}, + }, + "b": { + // No limits → falls back to requests. + Postgres: multigresv1alpha1.ContainerConfig{ + Resources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceMemory: resource.MustParse("2Gi"), + corev1.ResourceCPU: resource.MustParse("4"), + }, + }, + }, + Storage: multigresv1alpha1.StorageSpec{Size: "5Gi"}, + }, + }, + }, + } + mem, cpu, disk := reduceShardResources(shard) + if mem != 2*(1<<30) { + t.Errorf("mem = %d, want 2Gi (max, pool b request)", mem) + } + if cpu != 4000 { + t.Errorf("cpu = %d millicores, want 4000 (max, pool b request)", cpu) + } + if disk != 5*(1<<30) { + t.Errorf("disk = %d, want 5Gi (min across pools)", disk) + } + }, + ) + + t.Run("no pools returns zeros", func(t *testing.T) { + mem, cpu, disk := reduceShardResources(&multigresv1alpha1.Shard{}) + if mem != 0 || cpu != 0 || disk != 0 { + t.Errorf("got (%d, %d, %d), want all zero", mem, cpu, disk) + } + }) + + t.Run("pools without resources return zeros", func(t *testing.T) { + shard := &multigresv1alpha1.Shard{Spec: multigresv1alpha1.ShardSpec{ + Pools: map[multigresv1alpha1.PoolName]multigresv1alpha1.PoolSpec{"a": {}}, + }} + mem, cpu, disk := reduceShardResources(shard) + if mem != 0 || cpu != 0 || disk != 0 { + t.Errorf("got (%d, %d, %d), want all zero", mem, cpu, disk) + } + }) +} + +// A shard with no PostgresConfigRef still gets an operator-owned ConfigMap +// rendering the baseline — the operator always owns the file. +func TestReconcilePostgresConfig_RendersBaselineWithoutRef(t *testing.T) { + scheme := runtime.NewScheme() + _ = multigresv1alpha1.AddToScheme(scheme) + _ = corev1.AddToScheme(scheme) + + shard := &multigresv1alpha1.Shard{ + ObjectMeta: metav1.ObjectMeta{Name: "s1", Namespace: "default"}, + Spec: multigresv1alpha1.ShardSpec{}, + } + + c := fake.NewClientBuilder().WithScheme(scheme).Build() + r := &ShardReconciler{Client: c, Scheme: scheme} + + cfg := r.renderEffectiveConfig(context.Background(), shard) + if err := r.reconcilePostgresConfig(context.Background(), shard, cfg); err != nil { + t.Fatalf("reconcilePostgresConfig() error = %v", err) + } + + // The operator ConfigMap must exist with the rendered baseline. + got := &corev1.ConfigMap{} + if err := c.Get(context.Background(), client.ObjectKey{ + Namespace: "default", + Name: PostgresConfigMapName("s1"), + }, got); err != nil { + t.Fatalf("operator ConfigMap not created: %v", err) + } + rendered := got.Data[PostgresConfigMapKey] + if !strings.Contains(rendered, "shared_buffers = 64MB") { + t.Errorf("rendered baseline missing default shared_buffers:\n%s", rendered) + } + + // The content hash annotation must be stamped. + if len(shard.Annotations[metadata.AnnotationPostgresConfigHash]) != 64 { + t.Errorf("hash annotation = %q, want a 64-char SHA-256 hex", + shard.Annotations[metadata.AnnotationPostgresConfigHash]) + } +} + +// A shard with a PostgresConfigRef renders the baseline plus the ref content +// into the operator ConfigMap. +func TestReconcilePostgresConfig_MergesRefContent(t *testing.T) { + scheme := runtime.NewScheme() + _ = multigresv1alpha1.AddToScheme(scheme) + _ = corev1.AddToScheme(scheme) + + userCM := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: "user-cm", Namespace: "default"}, + Data: map[string]string{"custom.conf": "shared_buffers = '8GB'"}, + } + shard := &multigresv1alpha1.Shard{ + ObjectMeta: metav1.ObjectMeta{Name: "s1", Namespace: "default"}, + Spec: multigresv1alpha1.ShardSpec{ + PostgresConfigRef: &multigresv1alpha1.PostgresConfigRef{ + Name: "user-cm", + Key: "custom.conf", + }, + }, + } + + c := fake.NewClientBuilder().WithScheme(scheme).WithObjects(userCM).Build() + r := &ShardReconciler{Client: c, Scheme: scheme} + + cfg := r.renderEffectiveConfig(context.Background(), shard) + if err := r.reconcilePostgresConfig(context.Background(), shard, cfg); err != nil { + t.Fatalf("reconcilePostgresConfig() error = %v", err) + } + + got := &corev1.ConfigMap{} + if err := c.Get(context.Background(), client.ObjectKey{ + Namespace: "default", + Name: PostgresConfigMapName("s1"), + }, got); err != nil { + t.Fatalf("operator ConfigMap not created: %v", err) + } + rendered := got.Data[PostgresConfigMapKey] + // Baseline present, and the ref override appended after it. + if !strings.Contains(rendered, "shared_buffers = 64MB") { + t.Errorf("rendered config missing baseline:\n%s", rendered) + } + if !strings.Contains(rendered, "shared_buffers = '8GB'") { + t.Errorf("rendered config missing ref override:\n%s", rendered) + } + if strings.Index( + rendered, + "shared_buffers = '8GB'", + ) < strings.Index( + rendered, + "shared_buffers = 64MB", + ) { + t.Errorf("ref override should follow the baseline:\n%s", rendered) + } +} + +// The inline spec.postgresConfig map is rendered last, so it overrides both the +// baseline and any PostgresConfigRef content. +func TestReconcilePostgresConfig_InlineMapWins(t *testing.T) { + scheme := runtime.NewScheme() + _ = multigresv1alpha1.AddToScheme(scheme) + _ = corev1.AddToScheme(scheme) + + userCM := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: "user-cm", Namespace: "default"}, + Data: map[string]string{"custom.conf": "work_mem = '1MB'"}, + } + shard := &multigresv1alpha1.Shard{ + ObjectMeta: metav1.ObjectMeta{Name: "s1", Namespace: "default"}, + Spec: multigresv1alpha1.ShardSpec{ + PostgresConfigRef: &multigresv1alpha1.PostgresConfigRef{ + Name: "user-cm", + Key: "custom.conf", + }, + PostgresConfig: map[string]string{"work_mem": "64MB"}, + }, + } + + c := fake.NewClientBuilder().WithScheme(scheme).WithObjects(userCM).Build() + r := &ShardReconciler{Client: c, Scheme: scheme} + + cfg := r.renderEffectiveConfig(context.Background(), shard) + if err := r.reconcilePostgresConfig(context.Background(), shard, cfg); err != nil { + t.Fatalf("reconcilePostgresConfig() error = %v", err) + } + + got := &corev1.ConfigMap{} + if err := c.Get(context.Background(), client.ObjectKey{ + Namespace: "default", + Name: PostgresConfigMapName("s1"), + }, got); err != nil { + t.Fatalf("operator ConfigMap not created: %v", err) + } + rendered := got.Data[PostgresConfigMapKey] + if !strings.Contains(rendered, "work_mem = '64MB'") { + t.Errorf("rendered config missing inline override:\n%s", rendered) + } + // The inline map must appear after the ref content so it wins last-write-wins. + if strings.Index(rendered, "work_mem = '64MB'") < strings.Index(rendered, "work_mem = '1MB'") { + t.Errorf("inline map should follow the ref content:\n%s", rendered) + } +} diff --git a/pkg/resource-handler/controller/shard/shard_controller.go b/pkg/resource-handler/controller/shard/shard_controller.go index f72b2557..c91516cf 100644 --- a/pkg/resource-handler/controller/shard/shard_controller.go +++ b/pkg/resource-handler/controller/shard/shard_controller.go @@ -2,8 +2,6 @@ package shard import ( "context" - "crypto/sha256" - "encoding/hex" "fmt" "slices" "time" @@ -23,7 +21,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/controller" "sigs.k8s.io/controller-runtime/pkg/handler" "sigs.k8s.io/controller-runtime/pkg/log" - "sigs.k8s.io/controller-runtime/pkg/reconcile" multigresv1alpha1 "github.com/multigres/multigres-operator/api/v1alpha1" "github.com/multigres/multigres-operator/pkg/monitoring" @@ -129,13 +126,20 @@ func (r *ShardReconciler) Reconcile( return r.handlePendingDeletion(ctx, shard) } + // Render the effective postgresql.conf once per reconcile. Both the status + // path (drift detection) and the ConfigMap-delivery path below consume this + // single result, so the ref ConfigMap is fetched and the template rendered + // exactly once. A render/read error is reported softly in status here and + // then fails the reconcile at reconcilePostgresConfig below. + renderedCfg := r.renderEffectiveConfig(ctx, shard) + // Update status early so observedGeneration and pod-health phase are // always current. Later steps (especially reconcileDataPlane) may block // on the topo connection for an extended period; placing updateStatus // here guarantees the status subresource is written every reconcile. { _, childSpan := monitoring.StartChildSpan(ctx, "Shard.UpdateStatus") - if err := r.updateStatus(ctx, shard); err != nil { + if err := r.updateStatus(ctx, shard, renderedCfg); err != nil { monitoring.RecordSpanError(childSpan, err) childSpan.End() logger.Error(err, "Failed to update status") @@ -320,27 +324,20 @@ func (r *ShardReconciler) Reconcile( return ctrl.Result{}, err } - // Compute postgres config hash for rolling update detection. - // The hash is set as an in-memory annotation on the shard so that - // BuildPoolPod can propagate it to each pod without extra parameters. - if shard.Spec.PostgresConfigRef != nil { - configHash, err := r.computePostgresConfigHash(ctx, shard) - if err != nil { - logger.Error(err, "Failed to compute postgres config hash") - r.Recorder.Eventf( - shard, - "Warning", - "ConfigError", - "Failed to read postgres config ConfigMap %q: %v", - shard.Spec.PostgresConfigRef.Name, - err, - ) - return ctrl.Result{}, err - } - if shard.Annotations == nil { - shard.Annotations = make(map[string]string) - } - shard.Annotations[metadata.AnnotationPostgresConfigHash] = configHash + // Render the effective postgres config into the operator-owned ConfigMap and + // stamp a content hash for rolling update detection. The hash is set as an + // in-memory annotation on the shard so that BuildPoolPod can propagate it to + // each pod without extra parameters. + if err := r.reconcilePostgresConfig(ctx, shard, renderedCfg); err != nil { + logger.Error(err, "Failed to reconcile postgres config") + r.Recorder.Eventf( + shard, + "Warning", + "ConfigError", + "Failed to reconcile postgres config: %v", + err, + ) + return ctrl.Result{}, err } { @@ -642,52 +639,3 @@ func (r *ShardReconciler) SetupWithManager(mgr ctrl.Manager, opts ...controller. WithOptions(controllerOpts). Complete(r) } - -// computePostgresConfigHash fetches the referenced ConfigMap and returns a -// SHA-256 hex digest of the referenced key's data. This hash is placed on each -// pod as an annotation so that changes to the ConfigMap content produce a -// different spec-hash, triggering the existing rolling update mechanism. -func (r *ShardReconciler) computePostgresConfigHash( - ctx context.Context, - shard *multigresv1alpha1.Shard, -) (string, error) { - ref := shard.Spec.PostgresConfigRef - - cm := &corev1.ConfigMap{} - if err := r.Get(ctx, client.ObjectKey{ - Namespace: shard.Namespace, - Name: ref.Name, - }, cm); err != nil { - return "", fmt.Errorf("failed to get ConfigMap %q: %w", ref.Name, err) - } - - data, ok := cm.Data[ref.Key] - if !ok { - return "", fmt.Errorf("key %q not found in ConfigMap %q", ref.Key, ref.Name) - } - - sum := sha256.Sum256([]byte(data)) - return hex.EncodeToString(sum[:]), nil -} - -// enqueueFromPostgresConfigMap returns reconcile requests for Shards that -// reference the changed ConfigMap via spec.postgresConfigRef.name. -func (r *ShardReconciler) enqueueFromPostgresConfigMap( - ctx context.Context, - o client.Object, -) []reconcile.Request { - shards := &multigresv1alpha1.ShardList{} - if err := r.List(ctx, shards, client.InNamespace(o.GetNamespace())); err != nil { - return nil - } - - var requests []reconcile.Request - for _, s := range shards.Items { - if s.Spec.PostgresConfigRef != nil && s.Spec.PostgresConfigRef.Name == o.GetName() { - requests = append(requests, reconcile.Request{ - NamespacedName: client.ObjectKeyFromObject(&s), - }) - } - } - return requests -} diff --git a/pkg/resource-handler/controller/shard/shard_controller_internal_test.go b/pkg/resource-handler/controller/shard/shard_controller_internal_test.go index 6274da6e..c5a5e7a5 100644 --- a/pkg/resource-handler/controller/shard/shard_controller_internal_test.go +++ b/pkg/resource-handler/controller/shard/shard_controller_internal_test.go @@ -424,7 +424,7 @@ func TestUpdateStatus_PoolPodsNotFound(t *testing.T) { } // Call updateStatus when pool Pods don't exist yet - err := reconciler.updateStatus(context.Background(), shard) + err := reconciler.updateStatus(context.Background(), shard, renderedConfig{}) if err != nil { t.Errorf("updateStatus() should not error when pool Pods not found, got: %v", err) } @@ -781,7 +781,7 @@ func TestUpdateStatus_Multiorch(t *testing.T) { APIReader: fakeClient, } - err := reconciler.updateStatus(context.Background(), shard) + err := reconciler.updateStatus(context.Background(), shard, renderedConfig{}) if tc.expectError && err == nil { t.Error("updateStatus() should error but didn't") } @@ -854,7 +854,7 @@ func TestUpdateStatus_GetError(t *testing.T) { APIReader: fakeClient, } - err := reconciler.updateStatus(context.Background(), shard) + err := reconciler.updateStatus(context.Background(), shard, renderedConfig{}) if err == nil { t.Error("updateStatus() should error on Get failure") } @@ -927,7 +927,7 @@ func TestUpdateStatus_FieldOwner(t *testing.T) { APIReader: baseClient, } - err := reconciler.updateStatus(context.Background(), shard) + err := reconciler.updateStatus(context.Background(), shard, renderedConfig{}) if err != nil { t.Fatalf("updateStatus() unexpected error: %v", err) } @@ -4533,7 +4533,7 @@ func TestUpdateStatus_ProgressingPhase(t *testing.T) { recorder := record.NewFakeRecorder(10) r := &ShardReconciler{Client: c, Scheme: scheme, Recorder: recorder} - err := r.updateStatus(t.Context(), shard) + err := r.updateStatus(t.Context(), shard, renderedConfig{}) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -4574,7 +4574,8 @@ func TestUpdatePoolsStatus_PoolEmptyEvent(t *testing.T) { r := &ShardReconciler{Client: c, Scheme: scheme, Recorder: recorder} cellsSet := make(map[multigresv1alpha1.CellName]bool) - totalPods, readyPods, _, err := r.updatePoolsStatus(t.Context(), shard, cellsSet) + pools, err := r.updatePoolsStatus(t.Context(), shard, cellsSet, "") + totalPods, readyPods := pools.totalPods, pools.readyPods if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -4717,7 +4718,7 @@ func TestUpdateStatus_HealthyPhase(t *testing.T) { recorder := record.NewFakeRecorder(10) r := &ShardReconciler{Client: c, Scheme: scheme, Recorder: recorder} - err := r.updateStatus(t.Context(), shard) + err := r.updateStatus(t.Context(), shard, renderedConfig{}) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -4778,7 +4779,8 @@ func TestUpdatePoolsStatus_TerminatingPodExcluded(t *testing.T) { r := &ShardReconciler{Client: c, Scheme: scheme, Recorder: recorder} cellsSet := make(map[multigresv1alpha1.CellName]bool) - totalPods, readyPods, _, err := r.updatePoolsStatus(t.Context(), shard, cellsSet) + pools, err := r.updatePoolsStatus(t.Context(), shard, cellsSet, "") + totalPods, readyPods := pools.totalPods, pools.readyPods if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -4976,7 +4978,8 @@ func TestUpdatePoolsStatus_DrainAnnotationExcludedFromReady(t *testing.T) { r := &ShardReconciler{Client: c, Scheme: scheme, Recorder: recorder} cellsSet := make(map[multigresv1alpha1.CellName]bool) - totalPods, readyPods, _, err := r.updatePoolsStatus(t.Context(), shard, cellsSet) + pools, err := r.updatePoolsStatus(t.Context(), shard, cellsSet, "") + totalPods, readyPods := pools.totalPods, pools.readyPods if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -5058,7 +5061,7 @@ func TestUpdatePoolsStatus_DegradedOnCrashLoop(t *testing.T) { Build() r := &ShardReconciler{Client: c, Scheme: scheme, Recorder: record.NewFakeRecorder(10)} - if err := r.updateStatus(t.Context(), shard); err != nil { + if err := r.updateStatus(t.Context(), shard, renderedConfig{}); err != nil { t.Fatalf("unexpected error: %v", err) } if shard.Status.Phase != multigresv1alpha1.PhaseDegraded { @@ -5106,7 +5109,7 @@ func TestUpdatePoolsStatus_DegradedOnCrashLoop(t *testing.T) { Build() r := &ShardReconciler{Client: c, Scheme: scheme, Recorder: record.NewFakeRecorder(10)} - if err := r.updateStatus(t.Context(), s); err != nil { + if err := r.updateStatus(t.Context(), s, renderedConfig{}); err != nil { t.Fatalf("unexpected error: %v", err) } if s.Status.Phase != multigresv1alpha1.PhaseDegraded { @@ -5157,7 +5160,7 @@ func TestUpdatePoolsStatus_DegradedOnCrashLoop(t *testing.T) { Build() r := &ShardReconciler{Client: c, Scheme: scheme, Recorder: record.NewFakeRecorder(10)} - if err := r.updateStatus(t.Context(), s); err != nil { + if err := r.updateStatus(t.Context(), s, renderedConfig{}); err != nil { t.Fatalf("unexpected error: %v", err) } if s.Status.Phase == multigresv1alpha1.PhaseDegraded { @@ -5169,6 +5172,112 @@ func TestUpdatePoolsStatus_DegradedOnCrashLoop(t *testing.T) { }) } +// TestUpdatePoolsStatus_ConfigApplyFailing verifies the content-vs-health split +// in the config-rollout signal: a crash-looping pod that already carries the +// desired config hash marks the rollout as not-yet-settled (a bad GUC value +// Postgres rejects at startup looks exactly like this), while a crash-looping +// pod that carries a stale hash is only ordinary drift. +func TestUpdatePoolsStatus_ConfigApplyFailing(t *testing.T) { + scheme := runtime.NewScheme() + _ = multigresv1alpha1.AddToScheme(scheme) + _ = corev1.AddToScheme(scheme) + _ = appsv1.AddToScheme(scheme) + + shard := &multigresv1alpha1.Shard{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-shard-cfgfail", + Namespace: "default", + Labels: map[string]string{metadata.LabelMultigresCluster: "test-cluster"}, + }, + Spec: multigresv1alpha1.ShardSpec{ + DatabaseName: "db", + TableGroupName: "tg", + ShardName: "s1", + Pools: map[multigresv1alpha1.PoolName]multigresv1alpha1.PoolSpec{ + "primary": { + Cells: []multigresv1alpha1.CellName{"zone1"}, + ReplicasPerCell: ptr.To(int32(1)), + }, + }, + Multiorch: multigresv1alpha1.MultiorchSpec{ + Cells: []multigresv1alpha1.CellName{"zone1"}, + }, + }, + } + + labels := buildPoolLabelsWithCell(shard, "primary", "zone1") + podName := BuildPoolPodName(shard, "primary", "zone1", 0) + + const desiredHash = "desired-config-hash" + + crashLoopingPod := func(configHash string) *corev1.Pod { + return &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: podName, + Namespace: "default", + Labels: labels, + Annotations: map[string]string{ + metadata.AnnotationPostgresConfigHash: configHash, + }, + }, + Status: corev1.PodStatus{ + ContainerStatuses: []corev1.ContainerStatus{ + { + Name: "postgres", + State: corev1.ContainerState{ + Waiting: &corev1.ContainerStateWaiting{ + Reason: "CrashLoopBackOff", + }, + }, + }, + }, + }, + } + } + + t.Run("crash-looping pod on desired config is not settled", func(t *testing.T) { + c := fake.NewClientBuilder().WithScheme(scheme). + WithObjects(shard, crashLoopingPod(desiredHash)).Build() + r := &ShardReconciler{Client: c, Scheme: scheme, Recorder: record.NewFakeRecorder(10)} + + pools, err := r.updatePoolsStatus( + t.Context(), shard, make(map[multigresv1alpha1.CellName]bool), desiredHash, + ) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !pools.poolDegraded { + t.Error("expected poolDegraded=true for a crash-looping pod") + } + // The pod already carries the desired hash, so there is no content drift; + // configInProgress can only be true here via the apply-failing path (a + // desired-config pod crash-looping), which is exactly what must not settle. + if !pools.configInProgress { + t.Error("expected configInProgress=true: desired config is on a crash-looping pod") + } + }) + + t.Run("crash-looping pod on stale config is unsettled via drift", func(t *testing.T) { + c := fake.NewClientBuilder().WithScheme(scheme). + WithObjects(shard, crashLoopingPod("stale-hash")).Build() + r := &ShardReconciler{Client: c, Scheme: scheme, Recorder: record.NewFakeRecorder(10)} + + pools, err := r.updatePoolsStatus( + t.Context(), shard, make(map[multigresv1alpha1.CellName]bool), desiredHash, + ) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !pools.poolDegraded { + t.Error("expected poolDegraded=true for a crash-looping pod") + } + // The pod carries a stale hash, so config is unsettled via content drift. + if !pools.configInProgress { + t.Error("expected configInProgress=true: the pod carries a stale hash") + } + }) +} + func TestUpdateStatus_DegradedOnMultiorchCrashLoop(t *testing.T) { scheme := runtime.NewScheme() _ = multigresv1alpha1.AddToScheme(scheme) @@ -5254,7 +5363,7 @@ func TestUpdateStatus_DegradedOnMultiorchCrashLoop(t *testing.T) { Build() r := &ShardReconciler{Client: c, Scheme: scheme, Recorder: record.NewFakeRecorder(10)} - if err := r.updateStatus(t.Context(), shard); err != nil { + if err := r.updateStatus(t.Context(), shard, renderedConfig{}); err != nil { t.Fatalf("unexpected error: %v", err) } if shard.Status.Phase != multigresv1alpha1.PhaseDegraded { @@ -5605,12 +5714,12 @@ func TestEnqueueFromPostgresConfigMap(t *testing.T) { } } -func TestComputePostgresConfigHash(t *testing.T) { +func TestRenderEffectiveConfig_RefHashing(t *testing.T) { scheme := runtime.NewScheme() _ = multigresv1alpha1.AddToScheme(scheme) _ = corev1.AddToScheme(scheme) - t.Run("returns hash of referenced key", func(t *testing.T) { + t.Run("produces a deterministic hash over the rendered config", func(t *testing.T) { cm := &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{Name: "pg-config", Namespace: "default"}, Data: map[string]string{"custom.conf": "shared_buffers = '8GB'"}, @@ -5626,22 +5735,24 @@ func TestComputePostgresConfigHash(t *testing.T) { } c := fake.NewClientBuilder().WithScheme(scheme).WithObjects(cm).Build() - r := &ShardReconciler{Client: c} + r := &ShardReconciler{Client: c, Scheme: scheme} - hash, err := r.computePostgresConfigHash(context.Background(), shard) + rc := r.renderEffectiveConfig(context.Background(), shard) + hash, err := rc.hash, rc.err if err != nil { t.Fatalf("unexpected error: %v", err) } - if hash == "" { - t.Error("hash should not be empty") - } if len(hash) != 64 { t.Errorf("hash length = %d, want 64 (SHA-256 hex)", len(hash)) } - // Same content should produce same hash - hash2, _ := r.computePostgresConfigHash(context.Background(), shard) - if hash != hash2 { + // Same content should produce the same hash. + rc2 := r.renderEffectiveConfig(context.Background(), shard) + hash2, err := rc2.hash, rc2.err + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if hash2 != hash { t.Errorf("hash not deterministic: %q != %q", hash, hash2) } }) @@ -5657,7 +5768,7 @@ func TestComputePostgresConfigHash(t *testing.T) { } c := fake.NewClientBuilder().WithScheme(scheme).WithObjects(cm1, cm2).Build() - r := &ShardReconciler{Client: c} + r := &ShardReconciler{Client: c, Scheme: scheme} shard1 := &multigresv1alpha1.Shard{ ObjectMeta: metav1.ObjectMeta{Name: "s1", Namespace: "default"}, @@ -5678,8 +5789,16 @@ func TestComputePostgresConfigHash(t *testing.T) { }, } - h1, _ := r.computePostgresConfigHash(context.Background(), shard1) - h2, _ := r.computePostgresConfigHash(context.Background(), shard2) + rc1 := r.renderEffectiveConfig(context.Background(), shard1) + h1, err := rc1.hash, rc1.err + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + rc2 := r.renderEffectiveConfig(context.Background(), shard2) + h2, err := rc2.hash, rc2.err + if err != nil { + t.Fatalf("unexpected error: %v", err) + } if h1 == h2 { t.Error("different ConfigMap content should produce different hashes") } @@ -5701,11 +5820,11 @@ func TestComputePostgresConfigHash(t *testing.T) { } c := fake.NewClientBuilder().WithScheme(scheme).WithObjects(cm).Build() - r := &ShardReconciler{Client: c} + r := &ShardReconciler{Client: c, Scheme: scheme} - _, err := r.computePostgresConfigHash(context.Background(), shard) + err := r.renderEffectiveConfig(context.Background(), shard).err if err == nil { - t.Error("expected error for missing key") + t.Fatal("expected error for missing key") } if !strings.Contains(err.Error(), "missing-key") { t.Errorf("error should mention missing key, got: %v", err) @@ -5724,10 +5843,9 @@ func TestComputePostgresConfigHash(t *testing.T) { } c := fake.NewClientBuilder().WithScheme(scheme).Build() - r := &ShardReconciler{Client: c} + r := &ShardReconciler{Client: c, Scheme: scheme} - _, err := r.computePostgresConfigHash(context.Background(), shard) - if err == nil { + if r.renderEffectiveConfig(context.Background(), shard).err == nil { t.Error("expected error for missing ConfigMap") } }) diff --git a/pkg/resource-handler/controller/shard/shard_controller_test.go b/pkg/resource-handler/controller/shard/shard_controller_test.go index a99fda7b..2f41ca45 100644 --- a/pkg/resource-handler/controller/shard/shard_controller_test.go +++ b/pkg/resource-handler/controller/shard/shard_controller_test.go @@ -1304,7 +1304,7 @@ func TestShardReconciler_UpdateStatus(t *testing.T) { Recorder: record.NewFakeRecorder(100), } - if err := r.updateStatus(context.Background(), shard); err != nil { + if err := r.updateStatus(context.Background(), shard, renderedConfig{}); err != nil { t.Fatalf("updateStatus failed: %v", err) } @@ -1420,7 +1420,7 @@ func TestShardReconciler_UpdateStatus(t *testing.T) { Recorder: record.NewFakeRecorder(100), } - if err := r.updateStatus(context.Background(), shard); err != nil { + if err := r.updateStatus(context.Background(), shard, renderedConfig{}); err != nil { t.Fatalf("updateStatus failed: %v", err) } @@ -1528,12 +1528,13 @@ func TestShardReconciler_UpdateStatus(t *testing.T) { } cellsSet := make(map[multigresv1alpha1.CellName]bool) - totalPods, readyPods, _, err := r.updatePoolsStatus( - context.Background(), shard, cellsSet, + pools, err := r.updatePoolsStatus( + context.Background(), shard, cellsSet, "", ) if err != nil { t.Fatalf("updatePoolsStatus failed: %v", err) } + totalPods, readyPods := pools.totalPods, pools.readyPods // Verify aggregate: desired for primary is 3 pods per cell * 2 cells = 6 pods if totalPods != 6 { diff --git a/pkg/resource-handler/controller/shard/status.go b/pkg/resource-handler/controller/shard/status.go index 398366ac..410ea049 100644 --- a/pkg/resource-handler/controller/shard/status.go +++ b/pkg/resource-handler/controller/shard/status.go @@ -19,16 +19,21 @@ import ( "github.com/multigres/multigres-operator/pkg/util/status" ) -// updateStatus updates the Shard status based on observed state. +// updateStatus updates the Shard status based on observed state. rendered is the +// shard's effective-config render result (hash + any render/read error) computed +// once per reconcile by renderEffectiveConfig: the hash detects whether pods +// carry the current config, and a non-nil rendered.err (e.g. a missing +// PostgresConfigRef ConfigMap) is surfaced in the config status. func (r *ShardReconciler) updateStatus( ctx context.Context, shard *multigresv1alpha1.Shard, + rendered renderedConfig, ) error { oldPhase := shard.Status.Phase cellsSet := make(map[multigresv1alpha1.CellName]bool) // Update pools status - totalPods, readyPods, poolDegraded, err := r.updatePoolsStatus(ctx, shard, cellsSet) + pools, err := r.updatePoolsStatus(ctx, shard, cellsSet, rendered.hash) if err != nil { return err } @@ -43,15 +48,20 @@ func (r *ShardReconciler) updateStatus( shard.Status.Cells = cellSetToSlice(cellsSet) // Update aggregate status fields - shard.Status.PoolsReady = (totalPods > 0 && totalPods == readyPods) - shard.Status.ReadyReplicas = readyPods + shard.Status.PoolsReady = (pools.totalPods > 0 && pools.totalPods == pools.readyPods) + shard.Status.ReadyReplicas = pools.readyPods + + // Report config rollout state from the pool scan (content drift or a + // desired-config pod crash-looping; see poolStatus.configInProgress). A + // render/read error is reported without settling. + r.setPostgresConfigStatus(shard, pools.configInProgress, rendered.err) // Update Phase — Degraded takes priority over Healthy so crash-looping // pods are always surfaced even when the old replica is still serving. switch { - case poolDegraded || orchDegraded: + case pools.poolDegraded || orchDegraded: shard.Status.Phase = multigresv1alpha1.PhaseDegraded - if poolDegraded { + if pools.poolDegraded { shard.Status.Message = "One or more pool pods are crash-looping" } else { shard.Status.Message = "One or more Multiorch pods are crash-looping" @@ -69,7 +79,7 @@ func (r *ShardReconciler) updateStatus( } // Update conditions - r.setConditions(shard, totalPods, readyPods) + r.setConditions(shard, pools.totalPods, pools.readyPods) shard.Status.ObservedGeneration = shard.Generation @@ -104,6 +114,7 @@ func (r *ShardReconciler) updateStatus( LastBackupTime: shard.Status.LastBackupTime, LastBackupType: shard.Status.LastBackupType, PodRoles: shard.Status.PodRoles, + PostgresConfig: shard.Status.PostgresConfig, }, } @@ -135,18 +146,45 @@ func (r *ShardReconciler) updateStatus( return nil } -// updatePoolsStatus aggregates status from all pool pods. -// Returns total desired pods, ready pods, whether any pod is degraded (crash-looping), -// and tracks cells in the cellsSet. +// poolStatus is the aggregate of the pool-pod scan: pod counts, whether any pool +// pod is crash-looping (drives Phase=Degraded), and whether the rendered config +// has not yet settled on every pod (drives PostgresConfigStatus). Returning it as +// a struct keeps the pod-health and config-rollout signals from being smuggled +// through a positional multi-bool return. +type poolStatus struct { + totalPods, readyPods int32 + poolDegraded bool + + // configInProgress is true while the rendered config has not yet converged + // onto every live pod (content drift) or a pod already carrying the desired + // config is crash-looping. The latter is the narrow, config-attributable + // slice of pod health we intentionally couple to: a GUC value Postgres + // rejects at startup would otherwise be reported as applied. An unrelated + // crash-loop on a pod that does not carry the desired config does not affect + // config status (it still lacks the hash, so it reads as drift, not apply + // failure — either way the rollout is correctly reported as unsettled). + configInProgress bool +} + +// updatePoolsStatus aggregates status from all pool pods and tracks cells in the +// cellsSet. func (r *ShardReconciler) updatePoolsStatus( ctx context.Context, shard *multigresv1alpha1.Shard, cellsSet map[multigresv1alpha1.CellName]bool, -) (int32, int32, bool, error) { - var totalPods, readyPods int32 - var poolDegraded bool + desiredConfigHash string, +) (poolStatus, error) { clusterName := shard.Labels[metadata.LabelMultigresCluster] + var ps poolStatus + + // Track whether every live pod carries the desired config hash, and whether + // a pod that already carries it is crash-looping (config that Postgres + // rejects at startup manifests exactly this way). + liveCount := 0 + allConfigCurrent := true + configApplyFailing := false + for poolName, poolSpec := range shard.Spec.Pools { var poolDesired, poolReady int32 @@ -164,7 +202,7 @@ func (r *ShardReconciler) updatePoolsStatus( client.InNamespace(shard.Namespace), client.MatchingLabels(selector), ); err != nil { - return 0, 0, false, fmt.Errorf("failed to list pods for status: %w", err) + return poolStatus{}, fmt.Errorf("failed to list pods for status: %w", err) } var cellReady int32 @@ -183,9 +221,23 @@ func (r *ShardReconciler) updatePoolsStatus( continue } + // A live pod is config-current when it carries the desired hash. + liveCount++ + podHasDesiredConfig := pod.Annotations[metadata.AnnotationPostgresConfigHash] == desiredConfigHash + if !podHasDesiredConfig { + allConfigCurrent = false + } + // Detect crash-looping pods so the phase can escalate to Degraded. + // When the crash-looper already carries the desired config, treat + // the rollout as not-yet-settled: a bad GUC value fails Postgres + // startup here, and reporting such a config as applied would be a + // silent false positive. if status.IsCrashLooping(pod) { - poolDegraded = true + ps.poolDegraded = true + if podHasDesiredConfig { + configApplyFailing = true + } } // Check if pod is ready @@ -217,8 +269,8 @@ func (r *ShardReconciler) updatePoolsStatus( poolReady += cellReady } - totalPods += poolDesired - readyPods += poolReady + ps.totalPods += poolDesired + ps.readyPods += poolReady monitoring.SetShardPoolReplicas( clusterName, shard.Name, string(poolName), "", shard.Namespace, @@ -226,7 +278,15 @@ func (r *ShardReconciler) updatePoolsStatus( ) } - return totalPods, readyPods, poolDegraded, nil + // Content-based drift: at least one live pod carries a config hash other than + // the desired render. This is the "effective != desired" signal — it covers a + // spec edit, a PostgresConfigRef ConfigMap edit, and an operator-baseline + // change on upgrade alike, none of which the shard generation captures. When + // reload lands, the observed side migrates from "pod recreated with the hash" + // to "pgctld reports the loaded version"; the comparison stays the same. + configDrift := liveCount > 0 && !allConfigCurrent + ps.configInProgress = configDrift || configApplyFailing + return ps, nil } // updateMultiorchStatus checks Multiorch Deployments and sets OrchReady status. @@ -290,6 +350,42 @@ func (r *ShardReconciler) updateMultiorchStatus( return orchDegraded, nil } +// setPostgresConfigStatus records the rollout state of the rendered config on +// the shard status from the content-drift signal. inProgress is true while some +// pod still lacks the desired config; LastAppliedAt is (re)stamped only on the +// transition into "settled", so it stays stable while nothing is changing and +// advances for any change — including a PostgresConfigRef edit or a new operator +// baseline that never bumps the shard generation. A config error is reported +// without settling. +func (r *ShardReconciler) setPostgresConfigStatus( + shard *multigresv1alpha1.Shard, + inProgress bool, + configErr error, +) { + prev := shard.Status.PostgresConfig + st := &multigresv1alpha1.PostgresConfigStatus{} + if prev != nil { + st.LastAppliedAt = prev.LastAppliedAt + } + + switch { + case configErr != nil: + st.Error = configErr.Error() + case inProgress: + st.InProgress = true + default: + // Settled: stamp LastAppliedAt only when transitioning from an unsettled + // state, so a steady-state config does not churn the timestamp. + wasUnsettled := prev == nil || prev.InProgress || prev.Error != "" + if wasUnsettled { + now := metav1.Now() + st.LastAppliedAt = &now + } + } + + shard.Status.PostgresConfig = st +} + // cellSetToSlice converts a cell set (map) to a slice. func cellSetToSlice(cellsSet map[multigresv1alpha1.CellName]bool) []multigresv1alpha1.CellName { cells := make([]multigresv1alpha1.CellName, 0, len(cellsSet)) diff --git a/pkg/resource-handler/controller/shard/storage_class_guard_test.go b/pkg/resource-handler/controller/shard/storage_class_guard_test.go index 65d72e1e..7df20f97 100644 --- a/pkg/resource-handler/controller/shard/storage_class_guard_test.go +++ b/pkg/resource-handler/controller/shard/storage_class_guard_test.go @@ -384,7 +384,7 @@ func TestShardReconciler_FieldOwnershipIsolation(t *testing.T) { Recorder: record.NewFakeRecorder(100), } - if err := r.updateStatus(t.Context(), shard); err != nil { + if err := r.updateStatus(t.Context(), shard, renderedConfig{}); err != nil { t.Fatalf("updateStatus: %v", err) } diff --git a/pkg/webhook/handlers/defaulter.go b/pkg/webhook/handlers/defaulter.go index 16b05502..176a3a90 100644 --- a/pkg/webhook/handlers/defaulter.go +++ b/pkg/webhook/handlers/defaulter.go @@ -217,7 +217,7 @@ func (d *MultigresClusterDefaulter) Default(ctx context.Context, obj runtime.Obj if !isUsingTemplate { // We pass cell names for contextual cell defaulting, but leave // MaterializeCellDefaults false so the stored spec remains dynamic. - multiorchSpec, poolsSpec, resolvedPvcPolicy, resolvedBackupConfig, resolvedInitdbArgs, resolvedPostgresConfigRef, err := scopedResolver.ResolveShard( + resolved, err := scopedResolver.ResolveShard( ctx, shard, resolver.ResolveShardOptions{ @@ -231,21 +231,20 @@ func (d *MultigresClusterDefaulter) Default(ctx context.Context, obj runtime.Obj // Preserve PVCDeletionPolicy if it was set in the original spec // Otherwise, use the resolved policy (from template) - var pvcPolicy *multigresv1alpha1.PVCDeletionPolicy + pvcPolicy := resolved.PVCDeletionPolicy if shard.Spec != nil && shard.Spec.PVCDeletionPolicy != nil { pvcPolicy = shard.Spec.PVCDeletionPolicy - } else { - pvcPolicy = resolvedPvcPolicy } shard.Spec = &multigresv1alpha1.ShardInlineSpec{ - Multiorch: *multiorchSpec, - InitdbArgs: resolvedInitdbArgs, - PostgresConfigRef: resolvedPostgresConfigRef, - Pools: poolsSpec, + Multiorch: resolved.Multiorch, + InitdbArgs: resolved.InitdbArgs, + PostgresConfigRef: resolved.PostgresConfigRef, + PostgresConfig: resolved.PostgresConfig, + Pools: resolved.Pools, PVCDeletionPolicy: pvcPolicy, } - shard.Backup = resolvedBackupConfig + shard.Backup = resolved.Backup } } } diff --git a/pkg/webhook/handlers/validator.go b/pkg/webhook/handlers/validator.go index 6d20a40c..d7d28118 100644 --- a/pkg/webhook/handlers/validator.go +++ b/pkg/webhook/handlers/validator.go @@ -15,6 +15,7 @@ import ( multigresv1alpha1 "github.com/multigres/multigres-operator/api/v1alpha1" "github.com/multigres/multigres-operator/pkg/monitoring" + "github.com/multigres/multigres-operator/pkg/postgresconfig" "github.com/multigres/multigres-operator/pkg/resolver" "github.com/multigres/multigres-operator/pkg/util/metadata" ) @@ -103,7 +104,16 @@ func (v *MultigresClusterValidator) validate( childSpan.End() } - // 2. Deep Logic Validation (Safety Checks) + // 2. PostgreSQL parameter (GUC) validation. Runs before the deep-logic step + // because that step returns early when it emits warnings, which must not + // bypass rejecting invalid GUCs. + if err := validatePostgresConfig(cluster); err != nil { + monitoring.RecordSpanError(span, err) + monitoring.RecordWebhookRequest("VALIDATE", "MultigresCluster", err, time.Since(start)) + return nil, err + } + + // 3. Deep Logic Validation (Safety Checks) { _, childSpan := monitoring.StartChildSpan(ctx, "Webhook.ValidateLogic") if warnings, err := v.validateLogic(ctx, cluster); err != nil { @@ -186,6 +196,34 @@ func validateNoStorageShrink( return nil, nil } +// validatePostgresConfig checks every spec.postgresConfig map in the cluster +// (inline spec and overrides, at each shard) against the PostgreSQL parameter +// catalog, rejecting unknown names and gross type mismatches. +func validatePostgresConfig(cluster *multigresv1alpha1.MultigresCluster) error { + for _, db := range cluster.Spec.Databases { + for _, tg := range db.TableGroups { + for _, shard := range tg.Shards { + maps := []map[string]string{} + if shard.Spec != nil { + maps = append(maps, shard.Spec.PostgresConfig) + } + if shard.Overrides != nil { + maps = append(maps, shard.Overrides.PostgresConfig) + } + for _, m := range maps { + if err := postgresconfig.Validate(m); err != nil { + return fmt.Errorf( + "database %q tablegroup %q shard %q: %w", + db.Name, tg.Name, shard.Name, err, + ) + } + } + } + } + } + return nil +} + // collectPoolStorageSizes walks the cluster spec and returns a map of // "db/tg/shard/pool" → storage size string for all pools. func collectPoolStorageSizes(cluster *multigresv1alpha1.MultigresCluster) map[string]string { @@ -285,25 +323,26 @@ func NewTemplateValidator(c client.Client, kind string) *TemplateValidator { return &TemplateValidator{Client: c, Kind: kind} } -// ValidateCreate validates pool name map keys for ShardTemplates on creation. +// ValidateCreate validates ShardTemplates on creation (pool names, GUCs). func (v *TemplateValidator) ValidateCreate( ctx context.Context, obj runtime.Object, ) (admission.Warnings, error) { - return v.validatePoolNames(obj) + return v.validateShardTemplate(obj) } -// ValidateUpdate validates pool name map keys for ShardTemplates on update. +// ValidateUpdate validates ShardTemplates on update (pool names, GUCs). func (v *TemplateValidator) ValidateUpdate( ctx context.Context, oldObj, newObj runtime.Object, ) (admission.Warnings, error) { - return v.validatePoolNames(newObj) + return v.validateShardTemplate(newObj) } -// validatePoolNames validates pool name map keys for ShardTemplates. -// CRD structural schema does not enforce validation markers on map keys. -func (v *TemplateValidator) validatePoolNames(obj runtime.Object) (admission.Warnings, error) { +// validateShardTemplate validates a ShardTemplate's pool name map keys (the CRD +// structural schema does not enforce validation markers on map keys) and its +// postgresConfig parameters. +func (v *TemplateValidator) validateShardTemplate(obj runtime.Object) (admission.Warnings, error) { if v.Kind != "ShardTemplate" { return nil, nil } @@ -316,6 +355,9 @@ func (v *TemplateValidator) validatePoolNames(obj runtime.Object) (admission.War return nil, err } } + if err := postgresconfig.Validate(tpl.Spec.PostgresConfig); err != nil { + return nil, err + } return nil, nil } diff --git a/pkg/webhook/handlers/validator_test.go b/pkg/webhook/handlers/validator_test.go index bbd40498..2c344ab4 100644 --- a/pkg/webhook/handlers/validator_test.go +++ b/pkg/webhook/handlers/validator_test.go @@ -1355,3 +1355,90 @@ func TestMultigresClusterValidator_ValidateUpdate(t *testing.T) { } }) } + +func TestValidatePostgresConfig(t *testing.T) { + t.Parallel() + + cluster := func(spec, overrides map[string]string) *multigresv1alpha1.MultigresCluster { + sc := multigresv1alpha1.ShardConfig{Name: "0-inf"} + if spec != nil { + sc.Spec = &multigresv1alpha1.ShardInlineSpec{PostgresConfig: spec} + } + if overrides != nil { + sc.Overrides = &multigresv1alpha1.ShardOverrides{PostgresConfig: overrides} + } + return &multigresv1alpha1.MultigresCluster{ + Spec: multigresv1alpha1.MultigresClusterSpec{ + Databases: []multigresv1alpha1.DatabaseConfig{{ + Name: "postgres", + TableGroups: []multigresv1alpha1.TableGroupConfig{{ + Name: "default", + Shards: []multigresv1alpha1.ShardConfig{sc}, + }}, + }}, + }, + } + } + + tests := map[string]struct { + cluster *multigresv1alpha1.MultigresCluster + wantErr string + }{ + "no postgres config": {cluster: cluster(nil, nil)}, + "valid inline map": {cluster: cluster(map[string]string{"max_connections": "200"}, nil)}, + "valid overrides map": {cluster: cluster(nil, map[string]string{"work_mem": "16MB"})}, + "unknown param in inline": { + cluster: cluster(map[string]string{"maxx_connections": "200"}, nil), + wantErr: "unknown parameter", + }, + "bad value in overrides": { + cluster: cluster(nil, map[string]string{"fsync": "maybe"}), + wantErr: "fsync", + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + t.Parallel() + err := validatePostgresConfig(tc.cluster) + if tc.wantErr == "" { + if err != nil { + t.Errorf("expected nil, got %v", err) + } + } else if err == nil || !strings.Contains(err.Error(), tc.wantErr) { + t.Errorf("expected error containing %q, got %v", tc.wantErr, err) + } + // The error must identify the shard location. + if tc.wantErr != "" && err != nil && !strings.Contains(err.Error(), "shard") { + t.Errorf("error should name the shard, got %v", err) + } + }) + } +} + +func TestTemplateValidator_ShardTemplatePostgresConfig(t *testing.T) { + t.Parallel() + + fakeClient := fake.NewClientBuilder().WithScheme(setupScheme()).Build() + validator := NewTemplateValidator(fakeClient, "ShardTemplate") + + valid := &multigresv1alpha1.ShardTemplate{ + ObjectMeta: metav1.ObjectMeta{Name: "good"}, + Spec: multigresv1alpha1.ShardTemplateSpec{ + PostgresConfig: map[string]string{"max_connections": "200"}, + }, + } + if _, err := validator.ValidateCreate(t.Context(), valid); err != nil { + t.Errorf("valid postgresConfig rejected: %v", err) + } + + bad := &multigresv1alpha1.ShardTemplate{ + ObjectMeta: metav1.ObjectMeta{Name: "bad"}, + Spec: multigresv1alpha1.ShardTemplateSpec{ + PostgresConfig: map[string]string{"bogus_param": "1"}, + }, + } + if _, err := validator.ValidateUpdate(t.Context(), bad, bad); err == nil || + !strings.Contains(err.Error(), "unknown parameter") { + t.Errorf("expected unknown-parameter error, got %v", err) + } +} diff --git a/pkg/webhook/integration_test.go b/pkg/webhook/integration_test.go index 8de7bbbc..05850c53 100644 --- a/pkg/webhook/integration_test.go +++ b/pkg/webhook/integration_test.go @@ -308,6 +308,41 @@ func TestWebhook_Validation(t *testing.T) { t.Fatal("Expected error creating cluster with missing template, got nil") } }) + + t.Run("Should Reject Unknown Postgres Parameter", func(t *testing.T) { + cluster := &multigresv1alpha1.MultigresCluster{ + ObjectMeta: metav1.ObjectMeta{Name: "bad-guc", Namespace: testNamespace}, + Spec: multigresv1alpha1.MultigresClusterSpec{ + Cells: []multigresv1alpha1.CellConfig{{Name: "c", ZoneID: "use1-az1"}}, + Databases: []multigresv1alpha1.DatabaseConfig{{ + Name: "postgres", + Default: true, + TableGroups: []multigresv1alpha1.TableGroupConfig{{ + Name: "default", + Default: true, + Shards: []multigresv1alpha1.ShardConfig{{ + Name: "0-inf", + Spec: &multigresv1alpha1.ShardInlineSpec{ + PostgresConfig: map[string]string{"maxx_connections": "1"}, + Pools: map[multigresv1alpha1.PoolName]multigresv1alpha1.PoolSpec{ + "main": {Type: "readWrite"}, + }, + }, + }}, + }}, + }}, + }, + } + setTestPostgresPasswordSecretRef(cluster) + + err := k8sClient.Create(ctx, cluster) + if err == nil { + t.Fatal("expected rejection for unknown postgres parameter") + } + if !strings.Contains(err.Error(), "unknown parameter") { + t.Fatalf("expected 'unknown parameter' error, got: %v", err) + } + }) } func TestWebhook_TemplateProtection(t *testing.T) { diff --git a/test/e2e/framework/helpers.go b/test/e2e/framework/helpers.go index 377533d2..c91fb7b4 100644 --- a/test/e2e/framework/helpers.go +++ b/test/e2e/framework/helpers.go @@ -478,6 +478,87 @@ func WaitForQueryServing(t testing.TB, c *Cluster, ns, gatewaySvc string) { } } +// PsqlExec runs a single SQL statement through the gateway from a ready postgres +// pod and returns the trimmed result. ok is false (non-fatal) when no serving +// pod is found or psql fails, so callers can poll. sql must not contain single +// quotes (it is single-quoted into the shell command). +func (c *Cluster) PsqlExec(t testing.TB, ns, gwSvc, sql string) (string, bool) { + t.Helper() + ctx, cancel := context.WithTimeout(context.Background(), 20*time.Second) + defer cancel() + + pods, err := c.Clientset.CoreV1().Pods(ns).List(ctx, metav1.ListOptions{}) + if err != nil { + return "", false + } + var targetPod string + for _, pod := range pods.Items { + if pod.Status.Phase != corev1.PodRunning { + continue + } + // pgctld/postgres runs as a native sidecar (an init container with an + // always restart policy), so check both container lists. + for _, cs := range pod.Status.ContainerStatuses { + if cs.Name == "postgres" && cs.Ready { + targetPod = pod.Name + } + } + if targetPod == "" { + for _, cs := range pod.Status.InitContainerStatuses { + if cs.Name == "postgres" && cs.Ready { + targetPod = pod.Name + } + } + } + if targetPod != "" { + break + } + } + if targetPod == "" { + return "", false + } + + args := []string{ + "--kubeconfig", c.Kubeconfig, + "exec", "-n", ns, targetPod, "-c", "postgres", "--", + "sh", "-c", + fmt.Sprintf( + "PGPASSWORD=postgres psql -h %s -p 5432 -U postgres -d postgres -t -A -c '%s'", + gwSvc, sql, + ), + } + out, err := exec.CommandContext(ctx, "kubectl", args...).CombinedOutput() + if err != nil { + t.Logf("psql %q via %s: %v: %s", sql, gwSvc, err, strings.TrimSpace(string(out))) + return "", false + } + return strings.TrimSpace(string(out)), true +} + +// WaitForPsqlValue polls a query until it returns want, failing the test on +// timeout. Use it to assert an effective GUC value (e.g. "SHOW work_mem" → +// "16MB"), tolerating the brief window while a config change rolls out. +func WaitForPsqlValue(t testing.TB, c *Cluster, ns, gwSvc, sql, want string) { + t.Helper() + // Generous: a config change rolls out via a primary-last restart of every + // pool pod, which takes minutes on kind. Read-only asserts match on the + // first poll, so this ceiling only bites during a rollout. + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Minute) + defer cancel() + var last string + err := wait.PollUntilContextCancel(ctx, 5*time.Second, true, func(ctx context.Context) (bool, error) { + v, ok := c.PsqlExec(t, ns, gwSvc, sql) + if !ok { + return false, nil + } + last = v + return v == want, nil + }) + if err != nil { + t.Fatalf("query %q: got %q, want %q: %v", sql, last, want, err) + } +} + // --------------------------------------------------------------------------- // Mutation & assertion helpers // --------------------------------------------------------------------------- diff --git a/test/e2e/shared/postgresconfig/main_test.go b/test/e2e/shared/postgresconfig/main_test.go new file mode 100644 index 00000000..24d54dfc --- /dev/null +++ b/test/e2e/shared/postgresconfig/main_test.go @@ -0,0 +1,23 @@ +//go:build e2e + +package postgresconfig_test + +import ( + "fmt" + "os" + "testing" + + "github.com/multigres/multigres-operator/test/e2e/framework" +) + +var cluster *framework.Cluster + +func TestMain(m *testing.M) { + var err error + cluster, err = framework.EnsureSharedCluster() + if err != nil { + fmt.Fprintf(os.Stderr, "e2e setup: %v\n", err) + os.Exit(1) + } + os.Exit(m.Run()) +} diff --git a/test/e2e/shared/postgresconfig/postgresconfig_test.go b/test/e2e/shared/postgresconfig/postgresconfig_test.go new file mode 100644 index 00000000..bc021a2b --- /dev/null +++ b/test/e2e/shared/postgresconfig/postgresconfig_test.go @@ -0,0 +1,149 @@ +//go:build e2e + +package postgresconfig_test + +import ( + "context" + "strings" + "testing" + + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + + multigresv1alpha1 "github.com/multigres/multigres-operator/api/v1alpha1" + "github.com/multigres/multigres-operator/test/e2e/framework" +) + +// TestPostgresConfigManagement exercises the whole PostgreSQL-config feature +// end-to-end against a real cluster: the operator renders a baseline, scales it +// from resources, applies the inline spec.postgresConfig map (over the legacy +// ref), validates GUCs at admission, and rolls out + reports config changes. +// +// Every positive assertion reads the *effective* value with `SHOW ` +// through the gateway, so it proves the setting took effect in the running +// server — not merely that a ConfigMap has the right text. +func TestPostgresConfigManagement(t *testing.T) { + ns := cluster.CreateNamespace(t) + c, err := cluster.CRClient() + if err != nil { + t.Fatalf("create CR client: %v", err) + } + ctx := context.Background() + + // Legacy postgresConfigRef ConfigMap: sets random_page_cost (not in the + // inline map, to prove the ref is honored) and work_mem (which the inline map + // overrides, to prove precedence). + refCM := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: "pg-ref", Namespace: ns}, + Data: map[string]string{ + "custom.conf": "random_page_cost = '2.5'\nwork_mem = '64MB'", + }, + } + if err := c.Create(ctx, refCM); err != nil { + t.Fatalf("create ref ConfigMap: %v", err) + } + + cr := configCluster(ns) + if err := c.Create(ctx, cr); err != nil { + t.Fatalf("create MultigresCluster: %v", err) + } + + // Wait for Postgres to come up and serve queries. + framework.WaitForPod(t, c, ns, "postgres") + cluster.WaitForAllPodsReady(t, ns) + gw := framework.FindGatewayService(t, cluster, ns) + framework.WaitForQueryServing(t, cluster, ns, gw) + + t.Run("operator owns the baseline", func(t *testing.T) { + // wal_level comes from the operator's rendered baseline, not a user override. + framework.WaitForPsqlValue(t, cluster, ns, gw, "SHOW wal_level", "logical") + }) + + t.Run("config scales with pool resources", func(t *testing.T) { + // 512Mi memory limit → shared_buffers = mem/4 = 128MB (distinct from the + // static 64MB baseline, so this proves sizing is active). + framework.WaitForPsqlValue(t, cluster, ns, gw, "SHOW shared_buffers", "128MB") + }) + + t.Run("inline spec.postgresConfig is applied", func(t *testing.T) { + framework.WaitForPsqlValue(t, cluster, ns, gw, "SHOW max_connections", "150") + }) + + t.Run("inline map overrides the legacy ref", func(t *testing.T) { + // The ref sets work_mem=64MB; the inline map sets 16MB and must win. + framework.WaitForPsqlValue(t, cluster, ns, gw, "SHOW work_mem", "16MB") + }) + + t.Run("legacy postgresConfigRef is still honored", func(t *testing.T) { + // random_page_cost is only in the ref (not the map), so it must apply. + framework.WaitForPsqlValue(t, cluster, ns, gw, "SHOW random_page_cost", "2.5") + }) + + t.Run("operator renders a per-shard ConfigMap", func(t *testing.T) { + cms := &corev1.ConfigMapList{} + if err := c.List(ctx, cms, client.InNamespace(ns)); err != nil { + t.Fatalf("list ConfigMaps: %v", err) + } + found := false + for i := range cms.Items { + if strings.HasSuffix(cms.Items[i].Name, "-postgres-config") { + found = true + } + } + if !found { + t.Error("expected an operator-rendered -postgres-config ConfigMap") + } + }) + + // Changing spec.postgresConfig on a running cluster must actually roll out + // and take effect — not merely apply at initial creation. max_connections is + // a postmaster (restart-context) GUC, so this drives the primary-last restart + // path end to end and reads the effective value back from Postgres. + t.Run("changing spec.postgresConfig takes effect on the running cluster", func(t *testing.T) { + // Send the full databases array with the one changed value: JSON + // merge-patch replaces arrays wholesale, so we mutate the live object to + // preserve every server-defaulted field. + live := framework.GetCluster(t, c, ns, cr.Name) + live.Spec.Databases[0].TableGroups[0].Shards[0].Spec.PostgresConfig["max_connections"] = "200" + patch := framework.MustMarshal(map[string]any{ + "spec": map[string]any{"databases": live.Spec.Databases}, + }) + framework.PatchCluster(t, c, live, patch) + + // Poll the effective value until the rollout lands the change. + framework.WaitForPsqlValue(t, cluster, ns, gw, "SHOW max_connections", "200") + }) + + // Admission-time behavior (GUC validation rejection) is covered by the + // webhook envtest integration test — the e2e operator deployment does not + // install admission webhooks — and config-change rollout uses the same + // primary-last restart the scaling suite already exercises. +} + +// configCluster builds the inline sample with minimal CI resources, then sets a +// 512Mi memory limit on the postgres pool (for a deterministic shared_buffers) +// plus an inline postgresConfig map and the legacy postgresConfigRef. +func configCluster(ns string) *multigresv1alpha1.MultigresCluster { + cr := framework.MustLoadCluster("config/samples/no-templates.yaml", ns) + framework.WithCIResources(&cr.Spec) + + shard := &cr.Spec.Databases[0].TableGroups[0].Shards[0] + shard.Spec.PostgresConfig = map[string]string{ + "work_mem": "16MB", + "max_connections": "150", + } + shard.Spec.PostgresConfigRef = &multigresv1alpha1.PostgresConfigRef{ + Name: "pg-ref", + Key: "custom.conf", + } + for name, pool := range shard.Spec.Pools { + if pool.Postgres.Resources.Limits == nil { + pool.Postgres.Resources.Limits = corev1.ResourceList{} + } + pool.Postgres.Resources.Limits[corev1.ResourceMemory] = resource.MustParse("512Mi") + shard.Spec.Pools[name] = pool + } + return cr +}