ha-pg: dedicated walStorage 18Gi + primaryUpdateMethod switchover (REN-176) #1033

Merged

Ghost merged 2 commits from atlas/ren-176-ha-pg-walstorage into master 2026-09-27 09:00:58 +00:00

Conversation 3 Commits 2 Files Changed 1

+33

Ghost commented 2026-09-26 15:06:31 +00:00

Copy Link

Copy Source

What changed

kubernetes/apps/home-assistant/ha-pg-cluster.yaml only:

• walStorage: {storageClass: ceph-block, size: 21Gi}: a separate WAL PVC per instance (ha-pg-1-wal, ha-pg-2-wal).
• primaryUpdateMethod: switchover: the pod roll promotes the replica that already rolled. Without it the primary restarts in place (the CNPG default is restart).

Why

The owner approved option B on REN-176. The design and blast radius are in the REN-176 design document. This does not keep the recorder up when WAL fills, because a full WAL disk still stops Postgres. It keeps WAL writable if PGDATA fills, and WAL-volume usage tracks the archiver backlog, but no kubelet_volume_stats alert exists yet, so nothing pages on it (follow-up, not this PR). The 21Gi size gives ~4.1 days of full archiver outage ((21 − 1.86) / 4.66 GiB/day), against ~4.0 days on the shared 32Gi today (18.6 GiB free). 18Gi would give only ~3.5 days. The owner accepted B on the earlier "~4 days" wording; 21Gi is what makes that true. Sizing is in the manifest comment.

⚠️ Do not merge outside an owner-named maintenance window

Merging rolls both ha-pg pods and switches the primary over once. The recorder loses a few seconds of writes. Pre-conditions before merge:

• archiver queue is 0 .ready
• both instances are Ready
• a base backup completed within 24h
• not 02:00–02:30Z (the nightly WAL burst)

Verification

• Only the YAML is checked: it parses, and the spec has instances: 2, primaryUpdateMethod: switchover, storage 32Gi unchanged, and walStorage 21Gi ceph-block. The PGDATA storage comment now says WAL headroom moved to walStorage and 32Gi stays because a PVC cannot shrink.
• Not applied to a cluster and not server-side dry-run. This runner has no kubectl.
• Admission was checked against CNPG v1.30.1 source (REN-176 design §1):
  • validateWalStorageChange allows adding walStorage.
  • validateWalSizeConfiguration: 512MB < 2GB < 21Gi.
• Recommended before merge: kubectl diff by someone with cluster access.

Revert

There is no one-step revert. The CNPG webhook rejects removing walStorage ("walStorage cannot be disabled once configured"). A git revert would fail admission and leave the home-assistant Flux Kustomization stuck.

• Before merge: close this PR.
• After merge: the only way back is a new Cluster recovered from ha-pg-store, with the HA db_url repointed.
• If a pod fails mid-roll: delete that instance's pod and its PVCs, and the operator re-clones it (design §4).
• primaryUpdateMethod alone can be reverted normally.

🤖 Generated with Claude Code

## What changed `kubernetes/apps/home-assistant/ha-pg-cluster.yaml` only: - `walStorage: {storageClass: ceph-block, size: 21Gi}`: a separate WAL PVC per instance (`ha-pg-1-wal`, `ha-pg-2-wal`). - `primaryUpdateMethod: switchover`: the pod roll promotes the replica that already rolled. Without it the primary restarts in place (the CNPG default is `restart`). ## Why The owner approved option B on REN-176. The design and blast radius are in the REN-176 `design` document. This does **not** keep the recorder up when WAL fills, because a full WAL disk still stops Postgres. It keeps WAL writable if PGDATA fills, and WAL-volume usage tracks the archiver backlog, but no `kubelet_volume_stats` alert exists yet, so nothing pages on it (follow-up, not this PR). The 21Gi size gives ~4.1 days of full archiver outage ((21 − 1.86) / 4.66 GiB/day), against ~4.0 days on the shared 32Gi today (18.6 GiB free). 18Gi would give only ~3.5 days. The owner accepted B on the earlier "~4 days" wording; 21Gi is what makes that true. Sizing is in the manifest comment. ## ⚠️ Do not merge outside an owner-named maintenance window Merging rolls both `ha-pg` pods and switches the primary over once. The recorder loses a few seconds of writes. Pre-conditions before merge: - archiver queue is 0 `.ready` - both instances are Ready - a base backup completed within 24h - not 02:00–02:30Z (the nightly WAL burst) ## Verification - Only the YAML is checked: it parses, and the spec has `instances: 2`, `primaryUpdateMethod: switchover`, `storage` 32Gi unchanged, and `walStorage` 21Gi ceph-block. The PGDATA `storage` comment now says WAL headroom moved to `walStorage` and 32Gi stays because a PVC cannot shrink. - **Not applied to a cluster and not server-side dry-run.** This runner has no kubectl. - Admission was checked against CNPG v1.30.1 source (REN-176 design §1): - `validateWalStorageChange` allows adding `walStorage`. - `validateWalSizeConfiguration`: 512MB < 2GB < 21Gi. - Recommended before merge: `kubectl diff` by someone with cluster access. ## Revert **There is no one-step revert.** The CNPG webhook rejects removing `walStorage` ("walStorage cannot be disabled once configured"). A `git revert` would fail admission and leave the `home-assistant` Flux Kustomization stuck. - Before merge: close this PR. - After merge: the only way back is a new Cluster recovered from `ha-pg-store`, with the HA `db_url` repointed. - If a pod fails mid-roll: delete that instance's pod and its PVCs, and the operator re-clones it (design §4). - `primaryUpdateMethod` alone can be reverted normally. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

Ghost added 1 commit 2026-09-26 15:06:31 +00:00

ha-pg: add dedicated walStorage 18Gi, switch primary on update (REN-176) ... 68527e8244

Owner-approved option B on REN-176. Adds a separate ceph-block WAL volume
per instance and sets primaryUpdateMethod: switchover so the pod roll this
triggers promotes the rolled replica instead of restarting the primary in
place.

Permanent: the CNPG 1.30.1 webhook rejects removing walStorage, so a git
revert of this commit fails admission. Merge only in an owner-named window.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Ghost requested changes 2026-09-26 15:08:58 +00:00

Ghost left a comment

Copy Link

Copy Source

REQUEST_CHANGES — covers 68527e824411238d6544cb542e11a5f2ce9a30e6 only.

Blocking

1. Sizing claim is wrong, and the owner accepted on it. The comment and PR body say 18Gi keeps stall survival "no worse than the shared 32Gi (~4 days)". The comment's own sum adds only 3 days of outage. Free WAL room at 18Gi: (18 − 1.86) / 4.66 = 3.46 days. Today the shared volume has ~18.6 GiB free plus the 1.86 GiB pg_wal already in use, so ~20.5 GiB of WAL room ≈ 4.0 days. So 18Gi is ~0.5 day worse than today.
   Fix: either size: 21Gi ((21 − 1.86)/4.66 = 4.1 days). RBD is thin, so it costs the same nothing. Or keep 18Gi, change the comment and PR body to say "~3.5 days, below today's ~4", and take that back to the owner. I recommend 21Gi, with the comment arithmetic redone to match it.
2. Drift: the storage comment (lines 44–67) goes stale on merge. It says PGDATA is "sized for WAL headroom during an archiving stall". After this change WAL no longer lives there. Add one line: WAL headroom moves to walStorage (REN-176), and 32Gi stays because a PVC cannot shrink.

What I checked (with sources)

• Admission, CNPG v1.30.1 internal/webhook/v1/cluster_webhook.go:1748: validateWalStorageChange returns nil when old walStorage is nil (adding is allowed). Removing it is rejected with "walStorage cannot be disabled once configured". The docs (docs/src/storage.md:242) say the same. Permanent: confirmed.
• No re-clone needed: internal/cmd/manager/instance/run/cmd.go:345 calls ReconcileWalDirectory before Postgres starts.
• min_wal_size 512MB < max_wal_size 2GB (validateWalSizeConfiguration).
• WAL archiving survives the move: the barman-cloud plugin at v0.15.0 (the deployed version) copies every postgres-container volumeMount into the sidecar (internal/cnpgi/operator/lifecycle.go:586), so the sidecar sees the new WAL mount.
• primaryUpdateStrategy is unset, which defaults to unsupervised, so the switchover does not wait for a manual step.
• The diff is 1 file, +29/−0, based on the current master 0e2dea3e.

Not verified

• No server-side dry-run. My SA is denied get/patch on clusters.postgresql.cnpg.io in home-assistant (SelfSubjectAccessReview). Atlas has no kubectl either. Someone with cluster access has to run kubectl diff in the window. That is a merge precondition.

Non-blocking

• The PR says the WAL volume becomes "a direct archiver-backlog signal". No kubelet_volume_stats rule exists in this repo, so nothing alerts on it yet. That belongs on a follow-up, not this PR.

Merge is also gated on an owner-named maintenance window. A new commit voids this review, and I will re-review the new SHA in full.

**REQUEST_CHANGES — covers `68527e824411238d6544cb542e11a5f2ce9a30e6` only.** ## Blocking 1. **Sizing claim is wrong, and the owner accepted on it.** The comment and PR body say 18Gi keeps stall survival "no worse than the shared 32Gi (~4 days)". The comment's own sum adds only **3 days** of outage. Free WAL room at 18Gi: (18 − 1.86) / 4.66 = **3.46 days**. Today the shared volume has ~18.6 GiB free plus the 1.86 GiB pg_wal already in use, so ~20.5 GiB of WAL room ≈ **4.0 days**. So 18Gi is ~0.5 day *worse* than today. Fix: either **`size: 21Gi`** ((21 − 1.86)/4.66 = 4.1 days). RBD is thin, so it costs the same nothing. Or keep 18Gi, change the comment and PR body to say "~3.5 days, below today's ~4", and take that back to the owner. I recommend 21Gi, with the comment arithmetic redone to match it. 2. **Drift: the `storage` comment (lines 44–67) goes stale on merge.** It says PGDATA is "sized for WAL headroom during an archiving stall". After this change WAL no longer lives there. Add one line: WAL headroom moves to `walStorage` (REN-176), and 32Gi stays because a PVC cannot shrink. ## What I checked (with sources) - Admission, CNPG v1.30.1 `internal/webhook/v1/cluster_webhook.go:1748`: `validateWalStorageChange` returns nil when old `walStorage` is nil (adding is allowed). Removing it is rejected with "walStorage cannot be disabled once configured". The docs (`docs/src/storage.md:242`) say the same. **Permanent: confirmed.** - No re-clone needed: `internal/cmd/manager/instance/run/cmd.go:345` calls `ReconcileWalDirectory` before Postgres starts. - min_wal_size 512MB < max_wal_size 2GB (`validateWalSizeConfiguration`). - WAL archiving survives the move: the barman-cloud plugin at v0.15.0 (the deployed version) copies every postgres-container volumeMount into the sidecar (`internal/cnpgi/operator/lifecycle.go:586`), so the sidecar sees the new WAL mount. - `primaryUpdateStrategy` is unset, which defaults to unsupervised, so the switchover does not wait for a manual step. - The diff is 1 file, +29/−0, based on the current `master` `0e2dea3e`. ## Not verified - **No server-side dry-run.** My SA is denied `get`/`patch` on `clusters.postgresql.cnpg.io` in `home-assistant` (SelfSubjectAccessReview). Atlas has no kubectl either. Someone with cluster access has to run `kubectl diff` in the window. That is a merge precondition. ## Non-blocking - The PR says the WAL volume becomes "a direct archiver-backlog signal". No `kubelet_volume_stats` rule exists in this repo, so nothing alerts on it yet. That belongs on a follow-up, not this PR. Merge is also gated on an owner-named maintenance window. A new commit voids this review, and I will re-review the new SHA in full.

Ghost added 1 commit 2026-09-26 15:10:13 +00:00

ha-pg: walStorage 21Gi so stall survival matches today; note WAL left PGDATA (REN-176) ... b9a85474c0

Review #1114: 18Gi gave ~3.5 days of archiver outage, below the ~4.0 days
the shared 32Gi gives today. 21Gi gives ~4.1. The storage comment now says
WAL headroom moved to walStorage and 32Gi stays because PVCs cannot shrink.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Ghost approved these changes 2026-09-26 15:11:50 +00:00

Ghost left a comment

Copy Link

Copy Source

APPROVED — covers b9a85474c023326ee6e18c5a6e48344bc722da37 only. Approval is for content. It is not merge clearance: merge still needs an owner-named maintenance window, and a kubectl diff run in that window by someone with cluster access.

Blocking points from #1114: both fixed

1. Sizing. walStorage.size: 21Gi. Room for a stall is 21 − 1.86 = 19.14 GiB, and 19.14 / 4.66 = 4.1 days. Today it is 18.6 GiB free / 4.66 = 4.0 days. Both figures measure growth above the normal footprint, so they compare like with like. The nightly burst is already counted in the 4.66 GiB/day rate. So "about 4 days", the figure the owner accepted, is now true.
2. Drift. The PGDATA storage comment now says the stall headroom has moved to walStorage and that 32Gi stays because a PVC cannot shrink.

What I checked at this SHA

• git diff 68527e82 b9a85474: comment lines plus one value (18Gi → 21Gi). Nothing else changed.
• Parsed with yaml@2, with no errors: ha-pg has instances: 2, primaryUpdateMethod: switchover, storage ceph-block/32Gi, walStorage ceph-block/21Gi, min_wal_size 512MB < max_wal_size 2GB < 21Gi.
• master has moved to 3d6169ec (base 0e2dea3e). Nothing under kubernetes/apps/home-assistant/ changed. git merge-tree reports a clean merge.
• The admission, no-re-clone and archiver-mount findings from #1114 still hold. The files they cite are unchanged.

Non-blocking

• Manifest lines 81–82 still say WAL-volume usage is "a direct archiver-backlog signal". The PR body is now correct and says no alert exists. The manifest comment does not say that. Fix it in the follow-up that adds the kubelet_volume_stats rule. It does not need a new commit here.

A new commit voids this approval.

**APPROVED — covers `b9a85474c023326ee6e18c5a6e48344bc722da37` only.** Approval is for content. It is **not** merge clearance: merge still needs an owner-named maintenance window, and a `kubectl diff` run in that window by someone with cluster access. ## Blocking points from #1114: both fixed 1. **Sizing.** `walStorage.size: 21Gi`. Room for a stall is 21 − 1.86 = 19.14 GiB, and 19.14 / 4.66 = **4.1 days**. Today it is 18.6 GiB free / 4.66 = **4.0 days**. Both figures measure growth above the normal footprint, so they compare like with like. The nightly burst is already counted in the 4.66 GiB/day rate. So "about 4 days", the figure the owner accepted, is now true. 2. **Drift.** The PGDATA `storage` comment now says the stall headroom has moved to `walStorage` and that 32Gi stays because a PVC cannot shrink. ## What I checked at this SHA - `git diff 68527e82 b9a85474`: comment lines plus one value (`18Gi` → `21Gi`). Nothing else changed. - Parsed with `yaml@2`, with no errors: `ha-pg` has `instances: 2`, `primaryUpdateMethod: switchover`, storage `ceph-block/32Gi`, walStorage `ceph-block/21Gi`, `min_wal_size 512MB < max_wal_size 2GB < 21Gi`. - `master` has moved to `3d6169ec` (base `0e2dea3e`). Nothing under `kubernetes/apps/home-assistant/` changed. `git merge-tree` reports a clean merge. - The admission, no-re-clone and archiver-mount findings from #1114 still hold. The files they cite are unchanged. ## Non-blocking - Manifest lines 81–82 still say WAL-volume usage is "a direct archiver-backlog signal". The PR body is now correct and says no alert exists. The manifest comment does not say that. Fix it in the follow-up that adds the `kubelet_volume_stats` rule. It does not need a new commit here. A new commit voids this approval.

review-bot reviewed 2026-09-26 15:14:15 +00:00

review-bot left a comment

Member

Copy Link

Copy Source

🤖 Automated review — model CLI default (complexity: complex), commit b9a85474c023326ee6e18c5a6e48344bc722da37.

Advisory review by the review-bot pipeline. It never approves; a human merges. Push new commits to get a fresh review.

---

Summary

Single-file change to kubernetes/apps/home-assistant/ha-pg-cluster.yaml: adds a dedicated walStorage PVC (21Gi, ceph-block) and sets primaryUpdateMethod: switchover on the ha-pg CNPG cluster, with extensive comments explaining the sizing math and irreversibility. The YAML is well-formed, the field names match CNPG's documented Cluster spec, and the arithmetic in the comments checks out. No blocking issues found.

🟡 Suggestions

• PR title says 18Gi, the change ships 21Gi. The title reads "dedicated walStorage 18Gi + primaryUpdateMethod switchover", but ha-pg-cluster.yaml:97 sets size: 21Gi, and the body/comments explicitly reject 18Gi ("18Gi would give only ~3.5 days... 21Gi is what makes that true"). The title appears to be stale from an earlier revision. Given the PR body's own emphasis on care around this irreversible change, worth fixing the title before merge so it doesn't mislead anyone skimming PR history later.

• Unverifiable-from-repo admission/webhook claims. The body asserts specific CNPG v1.30.1 webhook behavior (validateWalStorageChange permits adding walStorage; validateWalSizeConfiguration requires min_wal_size < max_wal_size < walStorage.size; adding walStorage to an existing cluster triggers a pod roll where the instance manager moves pg_wal in place without a re-clone). This repo has no vendored CNPG source and I have no way to check the live operator version here, so I can't independently confirm these claims — they're plausible but unverified by this review. The PR's own recommendation to run kubectl diff (and ideally a dry run against a throwaway cluster/version-matched CNPG) before merging is the right mitigation given the stated one-way-door nature of this change (walStorage cannot be disabled once configured).

🟢 Nits

• ha-pg-cluster.yaml:22: "Since REN-176 WAL lives on walStorage below, so the stall headroom above now belongs to that volume." — the "Since... so..." construction is grammatically off (pick one); e.g. "Since REN-176, WAL lives on walStorage below, so the stall headroom above now belongs to that volume."

Tests

This is a Kubernetes manifest with no unit-test equivalent in this repo; .woodpecker/ci-manifests-test.yml's kustomize-build step will confirm the file still parses and the kustomization builds, but kubectl kustomize does not validate against the CNPG CRD/webhook, so it won't catch a malformed walStorage block or an admission rejection — that gap is exactly why the PR body recommends a manual kubectl diff/dry-run before merge. Given the explicit "no one-step revert" warning in the body, I'd treat that manual verification as a hard precondition rather than optional.

Recommendation

Looks reasonable — no blocking issues; comments above are optional (though I'd fix the stale "18Gi" in the PR title before merge for clarity).

🤖 **Automated review** — model `CLI default` (complexity: complex), commit `b9a85474c023326ee6e18c5a6e48344bc722da37`. > Advisory review by the `review-bot` pipeline. It never approves; a human merges. Push new commits to get a fresh review. --- ## Summary Single-file change to `kubernetes/apps/home-assistant/ha-pg-cluster.yaml`: adds a dedicated `walStorage` PVC (21Gi, `ceph-block`) and sets `primaryUpdateMethod: switchover` on the `ha-pg` CNPG cluster, with extensive comments explaining the sizing math and irreversibility. The YAML is well-formed, the field names match CNPG's documented `Cluster` spec, and the arithmetic in the comments checks out. No blocking issues found. ## 🟡 Suggestions - **PR title says 18Gi, the change ships 21Gi.** The title reads "dedicated walStorage 18Gi + primaryUpdateMethod switchover", but `ha-pg-cluster.yaml:97` sets `size: 21Gi`, and the body/comments explicitly reject 18Gi ("18Gi would give only ~3.5 days... 21Gi is what makes that true"). The title appears to be stale from an earlier revision. Given the PR body's own emphasis on care around this irreversible change, worth fixing the title before merge so it doesn't mislead anyone skimming PR history later. - **Unverifiable-from-repo admission/webhook claims.** The body asserts specific CNPG v1.30.1 webhook behavior (`validateWalStorageChange` permits adding `walStorage`; `validateWalSizeConfiguration` requires `min_wal_size < max_wal_size < walStorage.size`; adding `walStorage` to an existing cluster triggers a pod roll where the instance manager moves `pg_wal` in place without a re-clone). This repo has no vendored CNPG source and I have no way to check the live operator version here, so I can't independently confirm these claims — they're plausible but unverified by this review. The PR's own recommendation to run `kubectl diff` (and ideally a dry run against a throwaway cluster/version-matched CNPG) before merging is the right mitigation given the stated one-way-door nature of this change (`walStorage cannot be disabled once configured`). ## 🟢 Nits - `ha-pg-cluster.yaml:22`: "Since REN-176 WAL lives on walStorage below, so the stall headroom above now belongs to that volume." — the "Since... so..." construction is grammatically off (pick one); e.g. "Since REN-176, WAL lives on walStorage below, so the stall headroom above now belongs to that volume." ## Tests This is a Kubernetes manifest with no unit-test equivalent in this repo; `.woodpecker/ci-manifests-test.yml`'s `kustomize-build` step will confirm the file still parses and the kustomization builds, but `kubectl kustomize` does not validate against the CNPG CRD/webhook, so it won't catch a malformed `walStorage` block or an admission rejection — that gap is exactly why the PR body recommends a manual `kubectl diff`/dry-run before merge. Given the explicit "no one-step revert" warning in the body, I'd treat that manual verification as a hard precondition rather than optional. ## Recommendation Looks reasonable — no blocking issues; comments above are optional (though I'd fix the stale "18Gi" in the PR title before merge for clarity). <!-- pr-review:sha=b9a85474c023326ee6e18c5a6e48344bc722da37 -->

Ghost merged commit 8d5fc041bf into master 2026-09-27 09:00:58 +00:00

Ghost referenced this issue from a commit 2026-09-27 09:00:58 +00:00

Merge pull request 'ha-pg: dedicated walStorage 21Gi + primaryUpdateMethod switchover (REN-176)' (#1033)

Sign in to join this conversation.

Reviewers

No Reviewers

Ghost

review-bot

Labels

Clear labels

apollo

Hermes expert: Immich, Audiobookshelf, Libation, Navidrome, Music Assistant

atlas

Hermes expert: Talos, Flux, network, Authentik, Matrix, monitoring

clio

Hermes expert: Paperless, wiki, ical-proxy, TRMNL, websites

daedalus

Hermes expert: Gitea, Woodpecker, Renovate, review-bot, Hermes

hestia

Hermes expert: Home Assistant, IoT, MQTT, ESPHome, voice

No labels

Milestone

No items

No Milestone

Projects

Clear projects

No projects

Assignees

Clear assignees

alert-monitor-bot claude-bot (Claude Bot) flux hermes-bot issue-fix-bot k.renner renovate-bot review-bot (Review Bot) security-bot update-audit-bot

No Assignees

2 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: konairius/homelab#1033