konairius/homelab
10
0
Fork 0
Code Issues 9 Pull Requests 1 Packages Projects Releases 1 Wiki Activity

fix(ceph): add ceph-block-ephemeral SC for throwaway test PVCs (stop RBD leak) #416

Merged
k.renner merged 2 commits from ceph-block-ephemeral-sc into master 2026-07-26 07:07:17 +00:00
Conversation 2 Commits 2 Files Changed 3
+44 -9
claude-bot commented 2026-07-26 06:58:11 +00:00
Member
Copy Link
Copy Source

Fixes the daily RBD-image leak found while reclaiming 15 orphaned images (2026-07-26).

Root cause

Throwaway block PVCs used the default ceph-block StorageClass = reclaimPolicy: Retain. On PVC deletion the PV goes Released (RBD image kept), and the ad-hoc reclaim (patch reclaimPolicy=Delete → delete pv) races the ceph-csi provisioner — if it acts on the stale Retain policy it removes the PV but orphans the RBD image. Same class of bug as #262, which was fixed for CephFS (ceph-filesystem-ephemeral) but never for RBD.

Fix

  • Add ceph-block-ephemeral (reclaimPolicy: Delete) — the RBD mirror of the existing ceph-filesystem-ephemeral.
  • e2e-test.sh: point default/e2e-pvc at it — this was the ~1-image/day source (daily e2e crons daily-security-scan/update-audit).
  • RECOVERY.md: point the CNPG restore-test drill at it and simplify the teardown (Delete reclaim auto-removes PVC→PV→image; the manual patch dance is gone).

Verified

kubectl apply --dry-run creates ceph-block-ephemeral; bash -n e2e-test.sh OK.

Residual (separate, flagged)

The velero restore-test (Sundays 02:00) restores PVCs with the source's storageClass (ceph-block/Retain) and can't be repointed this way — velero's change-storage-class is global and would wrongly affect real DR restores. It still relies on purge_released_pvs. Worth a follow-up (harden that helper, or a velero restore-item-action scoped to the test).

Fixes the daily RBD-image leak found while reclaiming **15 orphaned images** (2026-07-26). ### Root cause Throwaway block PVCs used the default `ceph-block` StorageClass = `reclaimPolicy: Retain`. On PVC deletion the PV goes `Released` (RBD image kept), and the ad-hoc reclaim (`patch reclaimPolicy=Delete` → `delete pv`) **races the ceph-csi provisioner** — if it acts on the stale `Retain` policy it removes the PV but **orphans the RBD image**. Same class of bug as #262, which was fixed for CephFS (`ceph-filesystem-ephemeral`) but **never for RBD**. ### Fix - Add **`ceph-block-ephemeral`** (`reclaimPolicy: Delete`) — the RBD mirror of the existing `ceph-filesystem-ephemeral`. - **`e2e-test.sh`**: point `default/e2e-pvc` at it — this was the ~1-image/day source (daily e2e crons `daily-security-scan`/`update-audit`). - **`RECOVERY.md`**: point the CNPG restore-test drill at it and simplify the teardown (Delete reclaim auto-removes PVC→PV→image; the manual patch dance is gone). ### Verified `kubectl apply --dry-run` creates `ceph-block-ephemeral`; `bash -n e2e-test.sh` OK. ### Residual (separate, flagged) The **velero restore-test** (Sundays 02:00) restores PVCs with the *source's* storageClass (ceph-block/Retain) and can't be repointed this way — velero's `change-storage-class` is global and would wrongly affect real DR restores. It still relies on `purge_released_pvs`. Worth a follow-up (harden that helper, or a velero restore-item-action scoped to the test).
claude-bot added 1 commit 2026-07-26 06:58:11 +00:00
fix(ceph): add ceph-block-ephemeral SC for throwaway test PVCs (stop RBD leak)
ci/woodpecker/pr/pr-review Pipeline was successful
Details
ci/woodpecker/pr/ci-scripts-test Pipeline was successful
Details
b240a203cf 
Root cause of the daily RBD-image leak (found while reclaiming 15 orphaned
images on 2026-07-26): throwaway block PVCs used the default ceph-block
StorageClass, which is reclaimPolicy: Retain. On PVC deletion the PV goes
Released (image retained), and the ad-hoc reclaim (patch reclaimPolicy=Delete ->
delete pv) races the ceph-csi provisioner — if it acts on the stale Retain
policy it removes the PV but orphans the RBD image. Same class of bug as #262,
which was already fixed for CephFS via ceph-filesystem-ephemeral but never for
RBD.

- Add ceph-block-ephemeral (reclaimPolicy: Delete), the RBD mirror of the
  existing ceph-filesystem-ephemeral, in rook-ceph-cluster/storageclass.yaml.
- e2e-test.sh: point the throwaway default/e2e-pvc at it (this was the ~1
  image/day source via the daily e2e crons).
- RECOVERY.md: point the CNPG restore-test drill's throwaway cluster at it and
  simplify the teardown — with Delete reclaim, deleting the cluster removes the
  PVC -> PV -> RBD image automatically, so the manual PV-reclaim dance is gone.

Residual (separate): the velero restore-test (Sundays) restores PVCs with the
source's storageClass (ceph-block, Retain) and can't be repointed the same way;
it still relies on its purge_released_pvs helper.

Co-Authored-By: Claude Bot <claude-bot@renner.uno>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
review-bot left a comment
Member
Copy Link
Copy Source

🤖 Automated review — model claude-sonnet-5 (complexity: normal), commit b240a203cff515ac1ee869b449c67c74801950b0.

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

Summary

Adds a ceph-block-ephemeral StorageClass (RBD, reclaimPolicy: Delete) mirroring the existing ceph-filesystem-ephemeral pattern, and repoints the two known throwaway-PVC consumers (e2e-test.sh's e2e-pvc, and the CNPG restore-test drill in RECOVERY.md) at it, removing the racy manual patch reclaimPolicy=Delete + delete pv dance. This is a clean, low-risk, additive infra change — no blocking issues.

🟡 Suggestions

  • kubernetes/e2e-test.sh:192 — purge_released_pvs '^default/e2e-pvc$' is now dead weight for this PVC: since ceph-block-ephemeral reclaims on delete, there should never be a Released PV matching this pattern to purge. Harmless (the function no-ops on an empty list), but worth a comment or removal in a follow-up so a future reader doesn't wonder why it's still there for a class that no longer leaks.
  • kubernetes/RECOVERY.md:191-193 — the new comment explains the ephemeral class but doesn't restate the precondition mentioned in the residual note in the PR description (that this restore's PVC is genuinely throwaway and never a real DR target). Not required, but a one-line pointer to why it's safe to use Delete here specifically (vs. production CNPG storage) would help a reader who lands on this section without the PR context.

Tests

No automated test changes, appropriately — this is StorageClass/runbook/script config, not application logic. The PR notes kubectl apply --dry-run and bash -n e2e-test.sh were used to verify syntax; e2e-test.sh's existing ceph-block PVC provisions and pod writes+reads check (kubernetes/e2e-test.sh:180) will now implicitly exercise the new class on its next scheduled run, which is a reasonable de facto test for this change.

Recommendation

Looks reasonable — no blocking issues; comments above are optional.

🤖 **Automated review** — model `claude-sonnet-5` (complexity: normal), commit `b240a203cff515ac1ee869b449c67c74801950b0`. > Advisory review by the `review-bot` pipeline. It never approves; a human merges. Push new commits to get a fresh review. --- ## Summary Adds a `ceph-block-ephemeral` StorageClass (RBD, `reclaimPolicy: Delete`) mirroring the existing `ceph-filesystem-ephemeral` pattern, and repoints the two known throwaway-PVC consumers (`e2e-test.sh`'s `e2e-pvc`, and the CNPG restore-test drill in `RECOVERY.md`) at it, removing the racy manual `patch reclaimPolicy=Delete` + `delete pv` dance. This is a clean, low-risk, additive infra change — no blocking issues. ## 🟡 Suggestions - `kubernetes/e2e-test.sh:192` — `purge_released_pvs '^default/e2e-pvc$'` is now dead weight for this PVC: since `ceph-block-ephemeral` reclaims on delete, there should never be a Released PV matching this pattern to purge. Harmless (the function no-ops on an empty list), but worth a comment or removal in a follow-up so a future reader doesn't wonder why it's still there for a class that no longer leaks. - `kubernetes/RECOVERY.md:191-193` — the new comment explains the ephemeral class but doesn't restate the precondition mentioned in the residual note in the PR description (that this restore's PVC is genuinely throwaway and never a real DR target). Not required, but a one-line pointer to why it's safe to use `Delete` here specifically (vs. production CNPG storage) would help a reader who lands on this section without the PR context. ## Tests No automated test changes, appropriately — this is StorageClass/runbook/script config, not application logic. The PR notes `kubectl apply --dry-run` and `bash -n e2e-test.sh` were used to verify syntax; `e2e-test.sh`'s existing `ceph-block PVC provisions and pod writes+reads` check (kubernetes/e2e-test.sh:180) will now implicitly exercise the new class on its next scheduled run, which is a reasonable de facto test for this change. ## Recommendation Looks reasonable — no blocking issues; comments above are optional. <!-- pr-review:sha=b240a203cff515ac1ee869b449c67c74801950b0 -->
claude-bot added 1 commit 2026-07-26 07:04:58 +00:00
docs(ceph): address review nits on ceph-block-ephemeral (#416)
ci/woodpecker/pr/pr-review Pipeline was successful
Details
ci/woodpecker/pr/ci-scripts-test Pipeline was successful
Details
20e41ac289 
- e2e-test.sh: note that purge_released_pvs '^default/e2e-pvc$' is now a
  one-time safety net (e2e-pvc uses ceph-block-ephemeral, so it normally
  no-ops) rather than dead weight.
- RECOVERY.md: explain why Delete reclaim is safe *here* specifically — this is
  the throwaway restore-test, never a real DR target (a real cold-rebuild uses
  the live cluster's Retain ceph-block storage).

Co-Authored-By: Claude Bot <claude-bot@renner.uno>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
k.renner merged commit 4383f28dc1 into master 2026-07-26 07:07:17 +00:00
review-bot left a comment
Member
Copy Link
Copy Source

🤖 Automated review — model claude-sonnet-5 (complexity: normal), commit 20e41ac28912b81440f2fc26a6c2b7b507e44dd2.

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

Summary

Adds a ceph-block-ephemeral StorageClass (RBD, reclaimPolicy: Delete) as the RBD counterpart to the existing ceph-filesystem-ephemeral, and repoints two throwaway consumers (e2e-test.sh's e2e-pvc, and the CNPG restore-test drill in RECOVERY.md) at it, removing the manual PV-reclaim dance from the latter. This is a follow-up commit ("address review nits") on top of an earlier revision, and it reads as correct and low-risk. No blocking issues found.

The new StorageClass's parameters (clusterID, pool, imageFormat, imageFeatures, all six CSI secret refs, fstype) are byte-for-byte identical to the existing ceph-block class except for metadata.name and reclaimPolicy, so provisioning behavior for real workloads is unaffected — this only adds an opt-in alternate class. The RECOVERY.md teardown simplification (dropping the patch pv ...; delete pv dance) is consistent with existing CNPG behavior elsewhere in this repo: the previous comment ("deleting the cluster leaves a Released PV") already implied CNPG cascades PVC deletion when the Cluster is deleted, so with reclaimPolicy: Delete the PV/RBD image now goes away automatically — no owner-reference or GC dependency this PR introduces that isn't already relied on today.

🟡 Suggestions

  • kubernetes/e2e-test.sh:146 — the section header echo "=== Storage provisioning (ceph-block) ===" still says "ceph-block" even though the PVC it provisions now uses ceph-block-ephemeral (line 158). Cosmetic only, but slightly stale after this change.

Tests

No new automated test was added, but this PR modifies the test infrastructure itself in a way that is self-verifying: the next e2e-test.sh run and the next manual CNPG restore-test drill will exercise the new StorageClass end-to-end. The PR description notes kubectl apply --dry-run and bash -n e2e-test.sh were run manually — reasonable given this is declarative infra/ops tooling rather than application code, and a live cluster isn't available in this review environment to verify further.

Recommendation

Looks reasonable — no blocking issues; the comment above is optional.

🤖 **Automated review** — model `claude-sonnet-5` (complexity: normal), commit `20e41ac28912b81440f2fc26a6c2b7b507e44dd2`. > Advisory review by the `review-bot` pipeline. It never approves; a human merges. Push new commits to get a fresh review. --- ## Summary Adds a `ceph-block-ephemeral` StorageClass (RBD, `reclaimPolicy: Delete`) as the RBD counterpart to the existing `ceph-filesystem-ephemeral`, and repoints two throwaway consumers (`e2e-test.sh`'s `e2e-pvc`, and the CNPG restore-test drill in `RECOVERY.md`) at it, removing the manual PV-reclaim dance from the latter. This is a follow-up commit ("address review nits") on top of an earlier revision, and it reads as correct and low-risk. No blocking issues found. The new StorageClass's parameters (clusterID, pool, imageFormat, imageFeatures, all six CSI secret refs, fstype) are byte-for-byte identical to the existing `ceph-block` class except for `metadata.name` and `reclaimPolicy`, so provisioning behavior for real workloads is unaffected — this only adds an opt-in alternate class. The RECOVERY.md teardown simplification (dropping the `patch pv ...; delete pv` dance) is consistent with existing CNPG behavior elsewhere in this repo: the previous comment ("deleting the cluster leaves a Released PV") already implied CNPG cascades PVC deletion when the `Cluster` is deleted, so with `reclaimPolicy: Delete` the PV/RBD image now goes away automatically — no owner-reference or GC dependency this PR introduces that isn't already relied on today. ## 🟡 Suggestions - `kubernetes/e2e-test.sh:146` — the section header `echo "=== Storage provisioning (ceph-block) ==="` still says "ceph-block" even though the PVC it provisions now uses `ceph-block-ephemeral` (line 158). Cosmetic only, but slightly stale after this change. ## Tests No new automated test was added, but this PR modifies the test infrastructure itself in a way that is self-verifying: the next `e2e-test.sh` run and the next manual CNPG restore-test drill will exercise the new StorageClass end-to-end. The PR description notes `kubectl apply --dry-run` and `bash -n e2e-test.sh` were run manually — reasonable given this is declarative infra/ops tooling rather than application code, and a live cluster isn't available in this review environment to verify further. ## Recommendation Looks reasonable — no blocking issues; the comment above is optional. <!-- pr-review:sha=20e41ac28912b81440f2fc26a6c2b7b507e44dd2 -->
Sign in to join this conversation.
Reviewers
No Reviewers
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
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: konairius/homelab#416