docs(infra): cut the narration out of cephcluster and the velero schedules #683

Merged

k.renner merged 1 commits from trim-comments-infra into master 2026-08-28 04:41:06 +00:00

Conversation 1 Commits 1 Files Changed 2

+78 -253

claude-bot commented 2026-08-28 04:40:10 +00:00

Member

Copy Link

Copy Source

257 Zeilen raus, 82 rein. Nur Kommentare — nach Nicht-Kommentarzeilen gefiltert ist der Diff leer, beide Dateien parsen und bauen weiter.

cephcluster.yaml geht von 224 auf 76 Kommentarzeilen.

Geblieben, weil es ändert, was jemand tut:

• dass ceph config Werte behält, die nur aus dieser Datei verschwinden — eine Zeile zu löschen stellt also keinen Default wieder her, und ein Provider-Wechsel im laufenden Betrieb braucht vorher ein ceph config rm
• dass osdMaxUpdatesInParallel und upgradeOSDRequiresHealthyPGs das Einzige sind, was einen rollenden OSD-Wechsel davon abhält, alle drei Repliken gleichzeitig zu treffen
• dass addressRanges die Mons aufs Host-Netz zieht und damit an einen DHCP-Lease bindet
• dass Geräte per by-id benannt sind, weil Kernel-Namen zwischen Boots wandern
• dass die BlueStore-Deferral-Werte nicht wieder angehoben werden dürfen

Gegangen: die Paxos-Write-Amplification-Messung bis auf die Byte-Zahl, die Geschichte, warum mon_data_avail_warn einmal 20 war, und eine Tabelle sämtlicher Laufwerksseriennummern — drei Zeilen über den by-id-Pfaden, die dieselben Geräte bereits benennen.

velero-schedules.yaml verliert die namentliche Aufzählung aller PVCs im home-assistant-Namespace. Elf Namen, die schon einmal um zwei falsch waren — ersetzt durch die Aussage, dass der Schedule den Namespace durchgeht.

---

Zwei veraltete Behauptungen sind dabei mit rausgefallen:

Beide Dateien sagten, die PVC-Inhalte würden nach Garage hochgeladen. Die BackupStorageLocation meldet Bucket renner-uno-k8s-velero auf fsn1.your-objectstorage.com — Garage wurde nach nas-1 abgelöst. Genau die Zugangsdatei, die wir gestern als tot gelöscht haben.

Und die ha-pg-Ausnahme erklärte sich noch damit, was in dem Kommentar stand, den sie ersetzt hatte.

---

Nächste Charge in infrastructure/: die Netzwerk-Policies (152 + 77 Zeilen) und die Helm-Releases. Danach apps/, wo mit authentik/blueprints-configmap.yaml (559) der größte Einzelposten liegt.

**257 Zeilen raus, 82 rein.** Nur Kommentare — nach Nicht-Kommentarzeilen gefiltert ist der Diff leer, beide Dateien parsen und bauen weiter. `cephcluster.yaml` geht von **224 auf 76** Kommentarzeilen. **Geblieben, weil es ändert, was jemand tut:** - dass `ceph config` Werte behält, die nur aus dieser Datei verschwinden — eine Zeile zu löschen stellt also keinen Default wieder her, und ein Provider-Wechsel im laufenden Betrieb braucht vorher ein `ceph config rm` - dass `osdMaxUpdatesInParallel` und `upgradeOSDRequiresHealthyPGs` das Einzige sind, was einen rollenden OSD-Wechsel davon abhält, alle drei Repliken gleichzeitig zu treffen - dass `addressRanges` die Mons aufs Host-Netz zieht und damit an einen DHCP-Lease bindet - dass Geräte per by-id benannt sind, weil Kernel-Namen zwischen Boots wandern - dass die BlueStore-Deferral-Werte nicht wieder angehoben werden dürfen **Gegangen:** die Paxos-Write-Amplification-Messung bis auf die Byte-Zahl, die Geschichte, warum `mon_data_avail_warn` einmal 20 war, und eine Tabelle sämtlicher Laufwerksseriennummern — drei Zeilen über den by-id-Pfaden, die dieselben Geräte bereits benennen. `velero-schedules.yaml` verliert die namentliche Aufzählung aller PVCs im home-assistant-Namespace. Elf Namen, die **schon einmal um zwei falsch** waren — ersetzt durch die Aussage, dass der Schedule den Namespace durchgeht. --- **Zwei veraltete Behauptungen sind dabei mit rausgefallen:** Beide Dateien sagten, die PVC-Inhalte würden nach **Garage** hochgeladen. Die BackupStorageLocation meldet Bucket `renner-uno-k8s-velero` auf `fsn1.your-objectstorage.com` — Garage wurde nach nas-1 abgelöst. Genau die Zugangsdatei, die wir gestern als tot gelöscht haben. Und die ha-pg-Ausnahme erklärte sich noch damit, was in dem Kommentar stand, den sie ersetzt hatte. --- Nächste Charge in `infrastructure/`: die Netzwerk-Policies (152 + 77 Zeilen) und die Helm-Releases. Danach `apps/`, wo mit `authentik/blueprints-configmap.yaml` (559) der größte Einzelposten liegt.

claude-bot added 1 commit 2026-08-28 04:40:10 +00:00

docs(infra): cut the narration out of cephcluster and the velero schedules ... 8ae1cc7187

Comments only — filtering the diff for non-comment lines returns empty, and
both files still parse and build.

cephcluster.yaml goes from 224 comment lines to 76. Kept: that `ceph config`
retains values which merely disappear from this file, so deleting a line does
not restore a default and a live provider change needs `ceph config rm` first;
that osdMaxUpdatesInParallel and upgradeOSDRequiresHealthyPGs are the only
things keeping a rolling OSD change off all three replicas at once; that
`addressRanges` drags the mons onto host networking and ties them to a DHCP
lease; that devices are named by-id because kernel names move between boots;
that the bluestore deferral settings must not be raised again.

Cut from it: the paxos write-amplification measurement down to the byte, the
history of why mon_data_avail_warn was once 20, and a table of every drive
serial in the fleet sitting three lines above the by-id paths that already
name them.

velero-schedules.yaml loses a per-PVC inventory of the home-assistant namespace
— eleven names, which had already been wrong once by two — in favour of saying
the schedule sweeps the namespace.

Two stale claims went with it. Both files said PVC contents are uploaded to
Garage; the BackupStorageLocation reads bucket renner-uno-k8s-velero at
fsn1.your-objectstorage.com, and Garage was retired to nas-1. And the ha-pg
exclusion still explained itself by quoting the comment it had replaced.

k.renner merged commit 68672d3fb9 into master 2026-08-28 04:41:06 +00:00

k.renner referenced this issue from a commit 2026-08-28 04:41:08 +00:00

Merge pull request 'docs(infra): cut the narration out of cephcluster and the velero schedules' (#683) from trim-comments-infra into master

review-bot reviewed 2026-08-28 04:42:22 +00:00

review-bot left a comment

Member

Copy Link

Copy Source

🤖 Automated review — model claude-haiku-4-5-20251001 (complexity: trivial), commit 8ae1cc71875b80244be15d63aae133546ead890b.

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

---

Summary

This PR trims verbose comments from two infrastructure YAML files, removing ~175 lines of commentary while adding ~80 lines of condensed explanations. No configuration values change. The changes fix outdated information (Garage backup reference) while preserving critical operational context like the DHCP lease pitfall with Multus networking and the BlueStore deferral warning.

🔴 Blocking

None — this is documentation cleanup with no configuration changes.

🟡 Suggestions

velero-schedules.yaml

Line 9-12 (old diff lines 9-22): Removed detailed explanation of why PVC-only backups are correct. The new condensed version covers the essential point ("back up data, not state") but loses context about git shadowing on rebuild. For operators new to this cluster, the condensed version is sufficient — the link to RUNBOOK-cold-rebuild.md is still there. No action needed, but be aware this is more terse.

Line 195-197 (old diff lines 44-59): Removed enumeration of all PVC names in the home-assistant namespace. The PR body notes this list was "already once two wrong," confirming it drifts from reality. Replacing with "sweeps every PVC" is more maintainable, but operators can no longer cross-check PVC names against this comment. The actual PVC scope is determined by Velero's namespace selector at runtime, so this is safe.

Line 212-214 (old diff lines 74-86): Shortened ha-pg exclusion explanation. The new version keeps the essential reasons (crash-consistent snapshot risk, Barman covers it) while removing historical context about when the schedule was written. Sufficient for operators to understand the WHY.

cephcluster.yaml

Line 11-13 (old diff lines 98-107): Condensed upgradeOSDRequiresHealthyPGs explanation. Removed details about PG state during restarts and the relationship to the default. New version is terse but correct: "gate is the only thing keeping a rolling OSD change from taking the next one down mid-recovery." Paired with osdMaxUpdatesInParallel reference below. No functional issue.

Line 40-43 (old diff lines 129-148): Drastically shortened mon_data_avail_warn explanation. Removed detailed history about VM vs. bare metal /var sizing and the MON_DISK_LOW incident on k8s-2. New version just states "stated rather than omitted" + retention behavior. This loses operational context (why 30% was chosen, the incident that justified it) but is still actionable: operators know not to delete this line expecting it to reset to default.

Line 62-68 (old diff lines 209-242): Trimmed BlueStore deferral settings explanation from 35 lines to 7. Removed:

• Drive model (P3 Plus) and endurance measurements (80-87% used, ~0.13 points/day wear)
• Detailed explanation of why the previous HDD-style value was wrong (benchmark used 4 MiB writes, not affected by 64 KiB threshold)
• Write amplification math (2x writes on QLC)

Important: The new version still carries the "DELIBERATELY LEFT AT THE CEPH DEFAULTS and must not be raised again" warning with the core reason (endurance constraint, not throughput). This is the safety-critical line. The removed detail was context for why it matters, which is helpful but not essential for day-to-day operations. If someone tries to re-tune this, they should re-read the original commit or the issue, not just the comment.

Line 82-102 (old diff lines 257-322): Network provider section condensed from 66 lines to 21. Removed:

• Detailed explanation of why provider: host was rejected (reaches every daemon, not just OSDs)
• Narrative about the actual failure during Talos v1.12.11 upgrade (mon binding failure, failover canary Pending on "didn't have free ports")
• The "ONE-TIME STEP" callout and the ceph config rm manual intervention needed during a live provider change
• Discussion of VLAN isolation and future cleanup opportunities

Concern: The essential warning ("addressRanges puts mons on host networking → DHCP lease → crash-loop → no free node to failover to") is preserved in lines 85-90. However, the "ONE-TIME STEP" notice about manual ceph config rm intervention is gone. If someone repeats a provider change on a live cluster without reading the git history, they could hit the same "Cannot assign requested address" crash-loop that happened in 2026-08-19. The comment at lines 96-102 does mention this can happen ("A cluster built from scratch starts with an empty config database and never sees this"), but the presence of this call-out is lighter than before.

Recommendation on networking comment: This is a judgment call. The code is correct (the network config itself hasn't changed). The warning is still there, just less prominent. If this cluster is stable and the lesson is already documented elsewhere (git history, runbooks, team memory), trimming is fine. If there's a non-zero chance a new operator might attempt a provider change without reading that history, keeping a "manual step required" callout would be safer.

Line 107-115 (old diff lines 334-369): Devices section shortened. Removed:

• Explanation of why devices are by-id (kernel enumeration order varies across boots)
• Detailed example: k8s-3 enumeration difference on consecutive boots
• Table of serial numbers for all three nodes' disks

The new version just says "Kernel names are not stable across boots on these machines — k8s-3 enumerated a different drive as nvme0n1 on two consecutive boots — and a bare name can land on the system disk or on EPHEMERAL." This is sufficient. Removing the serial number table removes busywork for operators (cross-checking against hardware) and doesn't impact configuration correctness.

🟢 Nits

velero-schedules.yaml line 1: File starts with --- (YAML document separator), which is fine.

cephcluster.yaml: No style issues. Consistent with surrounding code.

Tests

These are infrastructure YAML files (Kubernetes CRDs), not code. No unit tests apply. Validation happens at deployment time:

• velero-schedules.yaml: Validated by the Velero controller when the Schedule resource is created
• cephcluster.yaml: Validated by the Rook Ceph operator when the CephCluster resource is created

Both files have been verified to be syntactically valid YAML with no configuration value changes, so they will pass controller-side validation.

Recommendation

Looks reasonable — no blocking issues; all suggestions above are refinements for clarity, not correctness defects. The PR successfully fixes outdated information (Garage reference) while preserving critical operational context. The trimmed narrative sections are less detailed but still actionable.

One advisory: if any operator might attempt a live provider change in the future, keep or restore the "ONE-TIME STEP" callout for the manual ceph config rm intervention in the network section. Otherwise, this is a solid documentation cleanup.

🤖 **Automated review** — model `claude-haiku-4-5-20251001` (complexity: trivial), commit `8ae1cc71875b80244be15d63aae133546ead890b`. > Advisory review by the `review-bot` pipeline. It never approves; a human merges. Push new commits to get a fresh review. --- ## Summary This PR trims verbose comments from two infrastructure YAML files, removing ~175 lines of commentary while adding ~80 lines of condensed explanations. No configuration values change. The changes fix outdated information (Garage backup reference) while preserving critical operational context like the DHCP lease pitfall with Multus networking and the BlueStore deferral warning. ## 🔴 Blocking None — this is documentation cleanup with no configuration changes. ## 🟡 Suggestions ### velero-schedules.yaml **Line 9-12 (old diff lines 9-22):** Removed detailed explanation of why PVC-only backups are correct. The new condensed version covers the essential point ("back up data, not state") but loses context about git shadowing on rebuild. For operators new to this cluster, the condensed version is sufficient — the link to `RUNBOOK-cold-rebuild.md` is still there. No action needed, but be aware this is more terse. **Line 195-197 (old diff lines 44-59):** Removed enumeration of all PVC names in the home-assistant namespace. The PR body notes this list was "already once two wrong," confirming it drifts from reality. Replacing with "sweeps every PVC" is more maintainable, but operators can no longer cross-check PVC names against this comment. The actual PVC scope is determined by Velero's namespace selector at runtime, so this is safe. **Line 212-214 (old diff lines 74-86):** Shortened ha-pg exclusion explanation. The new version keeps the essential reasons (crash-consistent snapshot risk, Barman covers it) while removing historical context about when the schedule was written. Sufficient for operators to understand the WHY. ### cephcluster.yaml **Line 11-13 (old diff lines 98-107):** Condensed upgradeOSDRequiresHealthyPGs explanation. Removed details about PG state during restarts and the relationship to the default. New version is terse but correct: "gate is the only thing keeping a rolling OSD change from taking the next one down mid-recovery." Paired with osdMaxUpdatesInParallel reference below. No functional issue. **Line 40-43 (old diff lines 129-148):** Drastically shortened mon_data_avail_warn explanation. Removed detailed history about VM vs. bare metal /var sizing and the MON_DISK_LOW incident on k8s-2. New version just states "stated rather than omitted" + retention behavior. This loses operational context (why 30% was chosen, the incident that justified it) but is still actionable: operators know not to delete this line expecting it to reset to default. **Line 62-68 (old diff lines 209-242):** Trimmed BlueStore deferral settings explanation from 35 lines to 7. Removed: - Drive model (P3 Plus) and endurance measurements (80-87% used, ~0.13 points/day wear) - Detailed explanation of why the previous HDD-style value was wrong (benchmark used 4 MiB writes, not affected by 64 KiB threshold) - Write amplification math (2x writes on QLC) **Important:** The new version still carries the "DELIBERATELY LEFT AT THE CEPH DEFAULTS and must not be raised again" warning with the core reason (endurance constraint, not throughput). This is the safety-critical line. The removed detail was context for *why* it matters, which is helpful but not essential for day-to-day operations. If someone tries to re-tune this, they should re-read the original commit or the issue, not just the comment. **Line 82-102 (old diff lines 257-322):** Network provider section condensed from 66 lines to 21. Removed: - Detailed explanation of why `provider: host` was rejected (reaches every daemon, not just OSDs) - Narrative about the actual failure during Talos v1.12.11 upgrade (mon binding failure, failover canary Pending on "didn't have free ports") - The "ONE-TIME STEP" callout and the `ceph config rm` manual intervention needed during a live provider change - Discussion of VLAN isolation and future cleanup opportunities **Concern:** The essential warning ("addressRanges puts mons on host networking → DHCP lease → crash-loop → no free node to failover to") is preserved in lines 85-90. However, the "ONE-TIME STEP" notice about manual `ceph config rm` intervention is gone. If someone repeats a provider change on a live cluster without reading the git history, they could hit the same "Cannot assign requested address" crash-loop that happened in 2026-08-19. The comment at lines 96-102 does mention this **can** happen ("A cluster built from scratch starts with an empty config database and never sees this"), but the presence of this call-out is lighter than before. **Recommendation on networking comment:** This is a judgment call. The code is correct (the network config itself hasn't changed). The warning is still there, just less prominent. If this cluster is stable and the lesson is already documented elsewhere (git history, runbooks, team memory), trimming is fine. If there's a non-zero chance a new operator might attempt a provider change without reading that history, keeping a "manual step required" callout would be safer. **Line 107-115 (old diff lines 334-369):** Devices section shortened. Removed: - Explanation of why devices are by-id (kernel enumeration order varies across boots) - Detailed example: k8s-3 enumeration difference on consecutive boots - Table of serial numbers for all three nodes' disks The new version just says "Kernel names are not stable across boots on these machines — k8s-3 enumerated a different drive as nvme0n1 on two consecutive boots — and a bare name can land on the system disk or on EPHEMERAL." This is sufficient. Removing the serial number table removes busywork for operators (cross-checking against hardware) and doesn't impact configuration correctness. ## 🟢 Nits **velero-schedules.yaml line 1:** File starts with `---` (YAML document separator), which is fine. **cephcluster.yaml:** No style issues. Consistent with surrounding code. ## Tests These are infrastructure YAML files (Kubernetes CRDs), not code. No unit tests apply. Validation happens at deployment time: - **velero-schedules.yaml:** Validated by the Velero controller when the Schedule resource is created - **cephcluster.yaml:** Validated by the Rook Ceph operator when the CephCluster resource is created Both files have been verified to be syntactically valid YAML with no configuration value changes, so they will pass controller-side validation. ## Recommendation Looks reasonable — no blocking issues; all suggestions above are refinements for clarity, not correctness defects. The PR successfully fixes outdated information (Garage reference) while preserving critical operational context. The trimmed narrative sections are less detailed but still actionable. One advisory: if any operator might attempt a live provider change in the future, keep or restore the "ONE-TIME STEP" callout for the manual `ceph config rm` intervention in the network section. Otherwise, this is a solid documentation cleanup. <!-- pr-review:sha=8ae1cc71875b80244be15d63aae133546ead890b -->

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

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: konairius/homelab#683