docs(dha): PoC finding — CNPG scale subresource has no selector; reopen actuation mechanism - #53
docs(dha): PoC finding — CNPG scale subresource has no selector; reopen actuation mechanism#53scooby87 wants to merge 1 commit into
Conversation
…en mechanism A live PoC validated the metric encoding (Σ+target drives the right count on real data) but found the load-bearing mechanism — a stock HPA via KEDA driving the CNPG Cluster scale subresource — is not buildable on stock CNPG: the Cluster /scale exposes no status.selector (no labelSelectorPath), so the HPA controller fails with InvalidSelector for every metric type. Upstream CNPG #7923 (add the selector) is closed as not planned; not fixable by a version bump. Record the finding and two resolution options — (A) KEDA + a thin mirror shim behind a proxy object with a selector, or (B) a lean actuation controller (no KEDA) writing Cluster.spec.instances directly. Note that neither returns rev1's ownership machinery, which §3 already removed. This reopens the actuation mechanism decision. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io>
📝 WalkthroughWalkthroughThe proposal records a PoC finding that prevents stock KEDA/HPA actuation for CloudNativePG. It reopens the mechanism decision and presents a mirror-shim approach and a direct controller approach. ChangesDatabase autoscaling proposal
Estimated code review effort: 1 (Trivial) | ~5 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
design-proposals/database-horizontal-autoscaling/README.md (2)
6-7: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winDo not present the superseded HPA path as the active design.
The status says that the mechanism is reopened, but the overview still describes direct KEDA/HPA-to-CNPG actuation and no new CRD. Option A adds a mirror shim and may add a CRD. Mark the later sections as the superseded pre-PoC design, or update the active overview and diagram after the mechanism decision.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@design-proposals/database-horizontal-autoscaling/README.md` around lines 6 - 7, Update the overview and architecture diagram in the database autoscaling proposal so they no longer present direct KEDA/HPA-to-CNPG actuation without a CRD as the active design. Either label the existing later sections as the superseded pre-PoC design, or revise them to reflect the selected post-PoC mechanism, including Option A’s mirror shim and possible CRD.
33-33: 🗄️ Data Integrity & Integration | 🔵 Trivial | 🏗️ Heavy liftDefine one concrete proxy contract for Option A.
A CRD proxy and a placeholder workload are not interchangeable. Specify the proxy type, scale fields, selector/status behavior, source of the desired count, observed CNPG count feedback, and retry/idempotency rules. A placeholder workload also creates real pods. The option is not testable until one proxy type and its ownership contract are selected.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@design-proposals/database-horizontal-autoscaling/README.md` at line 33, Define a single concrete proxy contract for Option A instead of presenting a CRD proxy and placeholder workload as alternatives. Specify the selected proxy type, ownership, scale fields, selector and status behavior, how the desired count is sourced, how observed CNPG instance counts feed back, and the retry/idempotency rules; if selecting a placeholder workload, document its pod implications. Update the Option A description so the design is concrete and testable.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@design-proposals/database-horizontal-autoscaling/README.md`:
- Line 34: Expand the Option B description to define fail-safe handling for
missing, NaN, stale, and timed-out VictoriaMetrics results: never interpret
invalid metrics as zero, specify freshness and query-timeout thresholds, and
state whether the controller retains the last known good value or performs no
scaling during outages. Also document how normal scaling resumes after valid
fresh metrics return, alongside the existing bounds and stabilization behavior.
- Around line 27-29: The blocker in the README incorrectly treats the missing
selector as version-independent. Update the discussion around the HPA/CNPG
scale-subresource limitation to reflect that CloudNativePG 1.30.0 provides
Cluster.status.selector and permits HPA for custom or custom-read-replica
metrics; either scope the claims to the CloudNativePG version range supported by
cozystack v45 or validate them with a PoC on 1.30.0+. Remove or qualify
unsupported statements about version bumps and the 1.29.2 selector fix while
preserving the documented limitations of the supported version.
---
Nitpick comments:
In `@design-proposals/database-horizontal-autoscaling/README.md`:
- Around line 6-7: Update the overview and architecture diagram in the database
autoscaling proposal so they no longer present direct KEDA/HPA-to-CNPG actuation
without a CRD as the active design. Either label the existing later sections as
the superseded pre-PoC design, or revise them to reflect the selected post-PoC
mechanism, including Option A’s mirror shim and possible CRD.
- Line 33: Define a single concrete proxy contract for Option A instead of
presenting a CRD proxy and placeholder workload as alternatives. Specify the
selected proxy type, ownership, scale fields, selector and status behavior, how
the desired count is sourced, how observed CNPG instance counts feed back, and
the retry/idempotency rules; if selecting a placeholder workload, document its
pod implications. Update the Option A description so the design is concrete and
testable.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 38bf1cfc-8bf1-4d8c-9093-8c523c1e079c
📒 Files selected for processing (1)
design-proposals/database-horizontal-autoscaling/README.md
| **The blocker.** That HPA never scales: it reports `ScalingActive=False, reason=InvalidSelector` — *"the HPA target's scale is missing a selector"*. The CNPG `Cluster` `/scale` subresource returns only `status: {replicas: N}` — no `status.selector` — and the CRD declares no `labelSelectorPath`. The Kubernetes HPA controller requires `scale.status.selector` unconditionally, before any metric-type branching, so this fails for **every** target type (confirmed with both `AverageValue` and `Value`). It is not a calibration detail and not fixable by a version bump: upstream CNPG issue [#7923](https://github.com/cloudnative-pg/cloudnative-pg/issues/7923), which requested exactly this selector for HPA/KEDA, is **closed as not planned**, and the CNPG 1.30 docs explicitly recommend against HPA for a `Cluster`. | ||
|
|
||
| **Consequence.** The load-bearing mechanism of this revision — a stock HPA (via KEDA) driving the CNPG `Cluster` scale subresource — cannot be built on stock CNPG; an actuation bridge is required after all. Crucially, what returns is **not** the machinery that got rev1 rejected: the ownership/enforcement layer (SSA, marker annotation, HelmRelease webhook, terminal-freeze) existed only because the chart declared `replicas`, and §3 (the chart omitting the field under autoscaling) removes it regardless of mechanism. What returns is only the small write-the-count actuator. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate relevant proposal files =="
git ls-files | grep -E 'design-proposals/database-horizontal-autoscaling|database-horizontal-autoscaling|Cozystack|cozystack|v45|45' || true
echo
echo "== proposal excerpt =="
if [ -f design-proposals/database-horizontal-autoscaling/README.md ]; then
sed -n '1,120p' design-proposals/database-horizontal-autoscaling/README.md | nl -ba
fi
echo
echo "== search for CNPG versions/support matrix mentions =="
rg -n "CNPG|CloudNativePG|cloudnative-pg|1\.2[7-9]|1\.30|Cozystack|v45|compat|supported version|version matrix|versions?|version-bump|HPA|HelmRelease|replicas|status\.selector|labelSelectorPath" . \
-g '!node_modules' -g '!dist' -g '!build' -g '!vendor' \
| head -n 300Repository: cozystack/community
Length of output: 318
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== proposal excerpt =="
sed -n '1,140p' design-proposals/database-horizontal-autoscaling/README.md
echo
echo "== version/matrix mentions =="
grep -RInE "CNPG|CloudNativePG|cloudnative-pg|1\.2[7-9]|1\.30|Cozystack|v45|compat|supported version|version matrix|versions|version-bump|HPA|HelmRelease|replicas|status\.selector|labelSelectorPath" . \
--exclude-dir=node_modules --exclude-dir=dist --exclude-dir=build --exclude-dir=vendor | head -n 400
echo
echo "== release note/docs snippets in repository files =="
grep -RInE "status\.selector|labelSelectorPath|HPA|recommended against|not planned|7923" . \
--exclude-dir=node_modules --exclude-dir=dist --exclude-dir=build --exclude-dir=vendor | head -n 200Repository: cozystack/community
Length of output: 50378
🌐 Web query:
cloudnative-pg 1.29 release notes status.selector HPA Cluster
💡 Result:
The status.selector field for the Cluster scale subresource, which enables integration with the Horizontal Pod Autoscaler (HPA) and Vertical Pod Autoscaler (VPA), was introduced in CloudNativePG 1.30.0, not 1.29.0 [1][2][3]. While CloudNativePG 1.29 included significant features such as dynamic network access control via podSelectorRefs and support for PostgreSQL extensions in image catalogs [4][5], the specific capability to expose a label selector via status.selector to allow HPA and VPA to map a Cluster to its instance pods was implemented in the subsequent 1.30.0 release (Pull Request #8996) [1][6][2].
Citations:
- 1: https://github.com/cloudnative-pg/cloudnative-pg/releases/tag/v1.30.0
- 2: https://github.com/cloudnative-pg/cloudnative-pg/blob/af12d5da/docs/src/release_notes/v1.30.md
- 3: https://newreleases.io/project/github/cloudnative-pg/cloudnative-pg/release/v1.30.0
- 4: https://github.com/cloudnative-pg/cloudnative-pg/releases/tag/v1.29.0
- 5: https://cloudnative-pg.io/releases/cloudnative-pg-1-29.0-released/
- 6: feat: add label selector to the Cluster scale subresource cloudnative-pg/cloudnative-pg#8996
🌐 Web query:
cloudnative-pg 1.30 HorizontalPodAutoscaler recommended against Cluster
💡 Result:
In CloudNativePG 1.30, the Cluster resource was updated to include a status.selector on its scale subresource, enabling it to be targeted by a Horizontal Pod Autoscaler (HPA) [1][2][3]. However, the official CloudNativePG documentation explicitly states that using an HPA to drive the number of instances in a Cluster is not recommended for PostgreSQL [4]. The recommendation against using HPA for a CNPG Cluster is based on the following technical reasons [4]: 1. Scaling Limitations: Increasing the number of instances in a Cluster only adds or removes standby replicas; it does not alleviate write load on the primary instance [4]. 2. Meaningless Metrics: The selector exposed by the scale subresource covers both the primary and all replicas [4]. Because these pods have fundamentally different workload profiles (write-heavy primary vs. read-only replicas), the per-pod average of CPU or memory metrics used by HPA is not a meaningful signal for scaling [4]. Adding replicas simply dilutes the average without addressing a potential bottleneck on the primary [4]. 3. Reconciliation Conflicts: HPA is generally unaware of CNPG’s internal constraints, such as synchronous replication requirements [4]. If HPA attempts to scale the cluster below the floor required by synchronous replicas (e.g., maxSyncReplicas + 1), the operator's validating webhook will reject the update, causing the HPA to continuously retry an unfulfillable state [4]. If you choose to use HPA despite these recommendations, the documentation advises that you must base the autoscaling on a custom metric that specifically reflects read-replica load and ensure that your minimum replica count (minReplicas) is set safely above the floor required by your synchronous replication configuration [4].
Citations:
- 1: https://cloudnative-pg.io/releases/cloudnative-pg-1-30.0-released/
- 2: https://github.com/cloudnative-pg/cloudnative-pg/blob/af12d5da/docs/src/release_notes/v1.30.md
- 3: feat: add label selector to the Cluster scale subresource cloudnative-pg/cloudnative-pg#8996
- 4: https://cloudnative-pg.io/docs/1.30/resource_management/
Correct the CNPG version assumptions in this blocker.
Cluster.status.selector is available in CloudNativePG 1.30.0, and CloudNativePG 1.30 documents HPA as not recommended but permitted only for custom/custom-read-replica metrics. Update the blocker to narrow the version-independent text to cozystack v45’s supported CloudNativePG range, or rerun the PoC against 1.30.0+; otherwise the “not fixable by a version bump” and “1.29.2 selector fix” claims are incorrect or unsupported.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@design-proposals/database-horizontal-autoscaling/README.md` around lines 27 -
29, The blocker in the README incorrectly treats the missing selector as
version-independent. Update the discussion around the HPA/CNPG scale-subresource
limitation to reflect that CloudNativePG 1.30.0 provides Cluster.status.selector
and permits HPA for custom or custom-read-replica metrics; either scope the
claims to the CloudNativePG version range supported by cozystack v45 or validate
them with a PoC on 1.30.0+. Remove or qualify unsupported statements about
version bumps and the 1.29.2 selector fix while preserving the documented
limitations of the supported version.
| **Two options to resolve (decision needed).** | ||
|
|
||
| - **Option A — KEDA + a thin mirror shim.** Keep KEDA's hardened decision loop by pointing its HPA at a proxy object that *does* expose a selector (a small owned CRD, or a placeholder workload), and add a tiny controller that mirrors the proxy's computed count into `Cluster.spec.instances`. Preserves the stock decision loop, but adds a shim, a proxy object, and the platform-wide KEDA dependency for a value KEDA cannot deliver end-to-end on its own. | ||
| - **Option B — a lean actuation controller, no KEDA.** A small controller reads the read-load metric from VictoriaMetrics and writes `Cluster.spec.instances` directly, applying the `min`/`max`/quorum-floor bounds and stabilization. This is close to rev1 **minus the ownership machinery** (which §3 already eliminates) and minus the aggregated-API enforcement — a much smaller component than the rejected operator, with no KEDA platform dependency, at the cost of a modest amount of stabilization logic KEDA would otherwise provide. |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Specify fail-safe metric handling for Option B.
If VictoriaMetrics returns no sample, NaN, stale data, or a timeout, the controller must not treat the result as zero. That behavior could scale CNPG to the quorum floor during a monitoring outage. Define freshness, timeout, last-known-good or no-scale behavior, and recovery behavior alongside bounds and stabilization.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@design-proposals/database-horizontal-autoscaling/README.md` at line 34,
Expand the Option B description to define fail-safe handling for missing, NaN,
stale, and timed-out VictoriaMetrics results: never interpret invalid metrics as
zero, specify freshness and query-timeout thresholds, and state whether the
controller retains the last known good value or performs no scaling during
outages. Also document how normal scaling resumes after valid fresh metrics
return, alongside the existing bounds and stabilization behavior.
1 participant
What this PR does
Amends the merged Database Horizontal Autoscaler proposal with a blocking finding from a live PoC, and puts two resolution options on the record for a decision. This follows up the design merged in #44.
The finding
A live PoC (cozystack v45, CloudNativePG 1.27.3, Kubernetes 1.34.3) validated the metric side of the design but found that the load-bearing mechanism — a stock HPA (via KEDA) driving the CNPG
Clusterscale subresource — cannot be built on stock CNPG.Σ + targetquery returns the right number on real data (151at target150→desired = ceil(151/150) = 2 = 1 primary + 1 read replica); the series/labels exist in VictoriaMetrics; the KEDA package installs and renders theScaledObjectinto a managed HPA.ScalingActive=False, reason=InvalidSelector. The CNPGCluster/scalesubresource exposes nostatus.selector(the CRD has nolabelSelectorPath), and the HPA controller requires the selector unconditionally — so it fails for every metric type (verified with bothAverageValueandValue). Upstream CNPG #7923 (add that selector for HPA/KEDA) is closed as not planned, and the CNPG 1.30 docs recommend against HPA for aCluster— so this is not fixable by a version bump.Why this is not a return to rev1
The machinery that got the first revision rejected — SSA ownership, the marker annotation, the HelmRelease webhook, terminal-freeze — existed only because the chart declared
replicas. §3 (the chart omitting the field under autoscaling) removes that regardless of mechanism. What the finding brings back is only a small write-the-count actuator, not the enforcement layer.Decision requested (Timofei Larkin (@lllamnyp))
Cluster.spec.instances. Adds a shim + proxy object + the platform-wide KEDA dependency.Cluster.spec.instancesdirectly with the min/max/quorum bounds and stabilization. Close to rev1 minus the ownership machinery (§3 already removes it), no KEDA dependency, at the cost of some stabilization logic.Both keep §1 (validated metric encoding) and §3 (chart omits the field). The choice is where the desired-count computation lives and whether to take KEDA as a platform dependency. Your Alternative 4 (a thin actuation shim) turns out to be needed for PostgreSQL too — not because it lacks a scale subresource, but because that subresource has no selector.
The implementation branch for the code (cozystack/cozystack#3381) is held — slices 1–3 (postgres ScaledObject + cozy-lib helper, KEDA package + wiring, alerts/dashboard) and the two-phase migration are done and green, but the actuation path is frozen pending this decision.
release-note
Summary by CodeRabbit