chore(rook-ceph): drop k8s-2 from the storage fleet for saturn's conversion #586

Merged
k.renner merged 1 commits from ceph-remove-k8s-2 into master 2026-08-25 20:07:29 +00:00
Member

Second of three bare-metal conversions. osd.0 is already out and evacuating.

Removing the node from storage.nodes before purging its OSD is what makes the purge stick. Under useAllNodes the operator rebuilt a purged OSD within one reconcile and started backfilling onto the drive being replaced — listing nodes explicitly exists precisely so a node can be taken out first. Merge this before I purge osd.0.

Re-add after the conversion naming the device by /dev/disk/by-id, as k8s-1 does. A bare name is not safe on bare metal: Talos enumerates the three NVMe drives differently from Proxmox and nvme0n1 is the system disk.

Pre-flight, measured

Converting jupiter changed the capacity picture enough that this window is far more comfortable than the last:

k8s-1 (bare metal) k8s-2 (VM) k8s-3 (VM)
CPU 20 8 8
Memory 94 GiB 47 GiB 47 GiB

The VM was capped at 8 vCPU / 48 GB; bare metal gets the full i9-13900H and all the RAM. Against k8s-2's 7,755m CPU and 32,112 Mi to rehome, k8s-1 alone has 16,520m and ~83 GiB free.

So immich stays up this time — the constraint that forced it down for jupiter is gone. cephfs-metadata remains at 2/1 as planned, and .mgr is dropped to 2/1 for the window for the same reason (at size 3 on two hosts it is undersized with min_size 2, so one further loss blocks the mgr).

Cluster was 217 active+clean with 3 mons and 3 OSDs before starting.

🤖 Generated with Claude Code

Second of three bare-metal conversions. `osd.0` is already `out` and evacuating. Removing the node from `storage.nodes` **before** purging its OSD is what makes the purge stick. Under `useAllNodes` the operator rebuilt a purged OSD within one reconcile and started backfilling onto the drive being replaced — listing nodes explicitly exists precisely so a node can be taken out first. Merge this before I purge `osd.0`. Re-add after the conversion naming the device by `/dev/disk/by-id`, as k8s-1 does. A bare name is not safe on bare metal: Talos enumerates the three NVMe drives differently from Proxmox and `nvme0n1` is the system disk. ## Pre-flight, measured Converting jupiter changed the capacity picture enough that this window is far more comfortable than the last: | | k8s-1 (bare metal) | k8s-2 (VM) | k8s-3 (VM) | |---|---|---|---| | CPU | **20** | 8 | 8 | | Memory | **94 GiB** | 47 GiB | 47 GiB | The VM was capped at 8 vCPU / 48 GB; bare metal gets the full i9-13900H and all the RAM. Against k8s-2's 7,755m CPU and 32,112 Mi to rehome, k8s-1 alone has 16,520m and ~83 GiB free. **So immich stays up this time** — the constraint that forced it down for jupiter is gone. `cephfs-metadata` remains at 2/1 as planned, and `.mgr` is dropped to 2/1 for the window for the same reason (at size 3 on two hosts it is undersized with min_size 2, so one further loss blocks the mgr). Cluster was 217 `active+clean` with 3 mons and 3 OSDs before starting. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
claude-bot added 1 commit 2026-08-25 20:03:00 +00:00
chore(rook-ceph): drop k8s-2 from the storage fleet for saturn's conversion
ci/woodpecker/pr/ci-scripts-test Pipeline was successful
ci/woodpecker/pr/ci-manifests-test Pipeline was successful
ci/woodpecker/pr/pr-review Pipeline was successful
ae6e013877
Removing the node from storage.nodes before purging its OSD is what makes the
purge stick. Under useAllNodes the operator rebuilt a purged OSD within one
reconcile and began backfilling onto the drive being replaced; listing nodes
explicitly exists so a node can be taken out first.

Re-add after the conversion naming the device by /dev/disk/by-id, as k8s-1
does. A bare device name is not safe on bare metal: Talos enumerates the three
NVMe drives in a different order than Proxmox, and nvme0n1 is the system disk.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
review-bot left a comment
Member

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

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


Summary

Removes the k8s-2 entry from cephcluster.yaml's storage.nodes list (kept as useAllNodes: false) and replaces it with a comment explaining why and how to re-add it after saturn's bare-metal conversion. This is step 1 of the documented RUNBOOK-baremetal-conversion.md procedure ("the node must already be out of cephcluster.yaml's storage.nodes list, or the operator rebuilds the OSD within one reconcile"). No blocking issues.

🟡 Suggestions

  • kubernetes/infrastructure/rook-ceph-cluster/cephcluster.yaml:188-191 — the new comment duplicates (in shorter form) the rationale already given at lines 163-179 in the same file and in the runbook's step 9. Not wrong, just a third copy of the same "why by-id, not bare name" explanation to keep in sync if the reasoning ever changes. Given this repo's existing style leans toward heavily-commented single-source explanations, you could shorten the new comment to "temporarily out for saturn's conversion, see runbook + comment above" — but this is a style call, not a defect.

Tests

Not applicable — this is a declarative Ceph CR change with no automated test harness in this repo; correctness here is operational (verified via ceph status/ceph osd tree per the runbook's step 10), consistent with how the rest of this file is handled.

Verification notes

  • Cross-checked against kubernetes/RUNBOOK-baremetal-conversion.md: this diff matches step 1 ("Evacuate the OSD") exactly, including the reason (avoiding the operator recreating a purged OSD via useAllNodes-like reconcile behavior even though useAllNodes is false here — it's storage.nodes membership, not that flag, that gates provisioning).
  • The PR body's claim that cephfs-metadata is already at size: 2 / min_size: 1 is correct and verified in kubernetes/infrastructure/rook-ceph-cluster/storageclass.yaml:110-114 — it was set for the multi-conversion window and is intentionally left untouched by this PR (confirmed by the runbook: "It stays at 2/1 across all three conversions rather than being toggled per node").
  • The .mgr pool size mentioned in the PR body is not tracked as a CR in this repo (Ceph creates it internally), so there's nothing in this diff to check for it; that adjustment is presumably applied imperatively as described.
  • Removing a node from storage.nodes does not itself purge/wipe the existing OSD deployment on that node (no cleanupPolicy is set in this CR) — it only stops the operator from re-provisioning it. This matches the PR body's framing ("Merge this before I purge osd.0") — the purge is a separate, deliberate follow-up step, not something this diff performs.
  • YAML structure and indentation of the new comment block are consistent with the surrounding file (6-space indent matching the sibling list items).

Recommendation

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

🤖 **Automated review** — model `CLI default` (complexity: complex), commit `ae6e01387792a7d8ac10b93426ad3c249896f9eb`. > Advisory review by the `review-bot` pipeline. It never approves; a human merges. Push new commits to get a fresh review. --- ## Summary Removes the `k8s-2` entry from `cephcluster.yaml`'s `storage.nodes` list (kept as `useAllNodes: false`) and replaces it with a comment explaining why and how to re-add it after saturn's bare-metal conversion. This is step 1 of the documented `RUNBOOK-baremetal-conversion.md` procedure ("the node must already be out of `cephcluster.yaml`'s `storage.nodes` list, or the operator rebuilds the OSD within one reconcile"). No blocking issues. ## 🟡 Suggestions - `kubernetes/infrastructure/rook-ceph-cluster/cephcluster.yaml:188-191` — the new comment duplicates (in shorter form) the rationale already given at lines 163-179 in the same file and in the runbook's step 9. Not wrong, just a third copy of the same "why by-id, not bare name" explanation to keep in sync if the reasoning ever changes. Given this repo's existing style leans toward heavily-commented single-source explanations, you could shorten the new comment to "temporarily out for saturn's conversion, see runbook + comment above" — but this is a style call, not a defect. ## Tests Not applicable — this is a declarative Ceph CR change with no automated test harness in this repo; correctness here is operational (verified via `ceph status`/`ceph osd tree` per the runbook's step 10), consistent with how the rest of this file is handled. ## Verification notes - Cross-checked against `kubernetes/RUNBOOK-baremetal-conversion.md`: this diff matches step 1 ("Evacuate the OSD") exactly, including the reason (avoiding the operator recreating a purged OSD via `useAllNodes`-like reconcile behavior even though `useAllNodes` is `false` here — it's `storage.nodes` membership, not that flag, that gates provisioning). - The PR body's claim that `cephfs-metadata` is already at `size: 2` / `min_size: 1` is correct and verified in `kubernetes/infrastructure/rook-ceph-cluster/storageclass.yaml:110-114` — it was set for the multi-conversion window and is intentionally left untouched by this PR (confirmed by the runbook: "It stays at 2/1 across all three conversions rather than being toggled per node"). - The `.mgr` pool size mentioned in the PR body is not tracked as a CR in this repo (Ceph creates it internally), so there's nothing in this diff to check for it; that adjustment is presumably applied imperatively as described. - Removing a node from `storage.nodes` does not itself purge/wipe the existing OSD deployment on that node (no `cleanupPolicy` is set in this CR) — it only stops the operator from re-provisioning it. This matches the PR body's framing ("Merge this before I purge `osd.0`") — the purge is a separate, deliberate follow-up step, not something this diff performs. - YAML structure and indentation of the new comment block are consistent with the surrounding file (6-space indent matching the sibling list items). ## Recommendation Looks reasonable — no blocking issues; comment above is optional. <!-- pr-review:sha=ae6e01387792a7d8ac10b93426ad3c249896f9eb -->
k.renner merged commit 269ccef5d2 into master 2026-08-25 20:07:29 +00:00
Sign in to join this conversation.