refactor(rook-ceph): drop the host network provider (phase A of two) #640

Merged
k.renner merged 1 commits from ceph-provider-phase-a into master 2026-08-26 19:54:07 +00:00
Member

Intermediate state, not a destination. san-network-attachment.yaml is already in the cluster and stays unused until phase B sets provider: multus.

The detour is forced, not chosen. Rook's CRD rejects the direct transition:

rule:    self == '' || oldSelf == '' || self == oldSelf
message: network provider must be disabled (reverted to empty string)
         before a new provider is enabled

host → multus is refused outright; it has to pass through empty. An empty provider means the default CNI, and addressRanges only takes effect under provider: host, so it goes with it.

Cost while this is in place. There is no cluster network. OSD heartbeat, replication and recovery move onto the pod network and share it with everything else instead of the dedicated X710 SFP+ link. Slower, and every OSD restart pays that in backfill.

Not a correctness problem: the OSDs can still reach one another throughout — which is the property that matters, and precisely the one a separate SAN subnet would have broken.

Mons do not move here. Rook leaves existing mons on the network they were created with; they change only on failover, which is deliberate and separate.

Bounded by what landed ahead of it (#639): one OSD restarts at a time, and only once PGs are clean. Expect three sequential OSD restarts with full recovery between them — and the same again in phase B.

Diff against the live cluster:

-  generation: 19
+  generation: 20
-    provider: host
+    provider: ""

The removal of addressRanges does not appear there: this resource carries no managedFields, so a client-side diff cannot see a field deletion. Inert either way — Rook does not read addressRanges without provider: host — but I will confirm it after the apply rather than assume it.

Phase B follows in a separate PR and must not be merged until this one has applied and the cluster is back to 217 PGs active+clean.

Intermediate state, **not a destination**. `san-network-attachment.yaml` is already in the cluster and stays unused until phase B sets `provider: multus`. **The detour is forced, not chosen.** Rook's CRD rejects the direct transition: ``` rule: self == '' || oldSelf == '' || self == oldSelf message: network provider must be disabled (reverted to empty string) before a new provider is enabled ``` `host` → `multus` is refused outright; it has to pass through empty. An empty provider means the default CNI, and `addressRanges` only takes effect under `provider: host`, so it goes with it. **Cost while this is in place.** There is no cluster network. OSD heartbeat, replication and recovery move onto the pod network and share it with everything else instead of the dedicated X710 SFP+ link. Slower, and every OSD restart pays that in backfill. Not a correctness problem: the OSDs can still reach one another throughout — which is the property that matters, and precisely the one a separate SAN subnet would have broken. **Mons do not move here.** Rook leaves existing mons on the network they were created with; they change only on failover, which is deliberate and separate. **Bounded by what landed ahead of it** (#639): one OSD restarts at a time, and only once PGs are clean. Expect three sequential OSD restarts with full recovery between them — and the same again in phase B. **Diff against the live cluster:** ``` - generation: 19 + generation: 20 - provider: host + provider: "" ``` The removal of `addressRanges` does not appear there: this resource carries no `managedFields`, so a client-side diff cannot see a field deletion. Inert either way — Rook does not read `addressRanges` without `provider: host` — but I will confirm it after the apply rather than assume it. Phase B follows in a separate PR and must not be merged until this one has applied and the cluster is back to 217 PGs `active+clean`.
claude-bot added 1 commit 2026-08-26 19:51:31 +00:00
refactor(rook-ceph): drop the host network provider (phase A of two)
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
a6dd1f4be2
Intermediate state, not a destination. san-network-attachment.yaml is already
in the cluster and stays unused until phase B sets provider: multus.

The detour is forced by Rook's own CRD validation, not chosen:

  rule:    self == '' || oldSelf == '' || self == oldSelf
  message: network provider must be disabled (reverted to empty string)
           before a new provider is enabled

host -> multus is rejected outright. It has to pass through empty, and an
empty provider means the default CNI — so addressRanges, which only takes
effect under provider: host, goes with it.

What this costs while it is in place: there is no cluster network. OSD
heartbeat, replication and recovery move onto the pod network and share it
with everything else instead of the dedicated X710 SFP+ link. Slower, and
every OSD restart pays that price in backfill. Not a correctness problem —
the OSDs can still reach one another throughout, which is the property that
matters, and the one a separate subnet would have broken.

The mons do not move here. Rook leaves existing mons on the network they were
created with; they change only on failover, which is deliberate and separate.

Bounded by the two settings that landed ahead of this: one OSD restarts at a
time, and only once PGs are clean again. Expect three sequential OSD restarts
with full recovery between them, and the same again in phase B.

kubectl diff against the live cluster shows provider host -> "" and the
generation bump. The removal of addressRanges does not appear there, because
this resource carries no managedFields and client-side diff cannot see a
field deletion. It is inert either way — Rook does not read addressRanges
without provider: host — but worth confirming after the apply rather than
assuming.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
k.renner merged commit e9f3175b5b into master 2026-08-26 19:54:07 +00:00
review-bot left a comment
Member

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

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


Summary

PR #640 is phase A of a two-phase migration of the Ceph cluster network off provider: host and onto Multus. It changes kubernetes/infrastructure/rook-ceph-cluster/cephcluster.yaml to set network.provider: "" (from host) and drops the now-inert addressRanges block, forced by Rook's CRD validation rule that rejects a direct host → multus transition. The change itself is small, well-scoped, and matches the constraints it describes — I verified the referenced files (san-network-attachment.yaml exists and is unused, osdMaxUpdatesInParallel: 1 and upgradeOSDRequiresHealthyPGs: true are already in place from #639) and the reasoning is internally consistent. No blocking issues found in the diff itself; one operational risk worth confirming before/at apply time is called out below.

🟡 Suggestions

  • kubernetes/infrastructure/network-policies/rook-ceph-ns.yaml:9-12,33-35 — this CiliumNetworkPolicy states as a load-bearing assumption that "everything ceph-daemon-y (mons, OSDs, MDSs, RGW, ... mgr, exporter, crashcollector) runs on hostNetwork and is OUTSIDE Cilium endpoint policy entirely." network.provider: host is exactly what makes that true today, and this PR sets it to "". OSDs restart as part of this same PR (per its own description), so they — and likely the mgr deployment (count: 2), which reconciles without a restart gate — will come back on the pod network and become real CiliumEndpoints subject to this policy, not exempt from it.

    Tracing the existing rules, this probably still works: OSD↔OSD and mgr↔OSD traffic is intra-namespace and covered by the fromEndpoints: namespace=rook-ceph rule, and mon-originated traffic (mons stay on hostNetwork per the PR) should still be classified under the fromEntities: [host, remote-node] rule regardless of the destination pod's network mode. But I could not confirm Cilium's identity classification for host-originated traffic hitting a pod-network endpoint against a live cluster, and this is the one place where being wrong is a full storage-cluster outage, not a degraded state. Worth an explicit check right after applying (e.g. watch OSD peering/heartbeat in ceph status, or cilium monitor --type drop -n rook-ceph during/after the OSD restarts) rather than assuming the policy tolerates the transition.

    Separately, the comment itself is now inaccurate for OSDs/mgr once this lands — worth a follow-up update so it doesn't mislead the next reader into thinking Ceph daemons are still uniformly hostNetwork-exempt.

Tests

No automated test can validate this kind of change — it's a live-cluster behavioral edit that kustomize-build in ci-manifests-test.yml will only check for YAML/kustomize structural validity, not runtime effect. That's an accurate limitation the PR itself acknowledges (it states the author will confirm the addressRanges removal against the live object after applying, since the resource carries no managedFields). No gap to flag beyond the NetworkPolicy interaction above, which is also not something CI can catch.

Recommendation

Looks reasonable — no blocking issues; comment above is a pre-apply verification worth doing, not a reason to hold the PR.

🤖 **Automated review** — model `CLI default` (complexity: complex), commit `a6dd1f4be21b7dbb3cd988458ac943ecee961b2b`. > Advisory review by the `review-bot` pipeline. It never approves; a human merges. Push new commits to get a fresh review. --- ## Summary PR #640 is phase A of a two-phase migration of the Ceph cluster network off `provider: host` and onto Multus. It changes `kubernetes/infrastructure/rook-ceph-cluster/cephcluster.yaml` to set `network.provider: ""` (from `host`) and drops the now-inert `addressRanges` block, forced by Rook's CRD validation rule that rejects a direct `host` → `multus` transition. The change itself is small, well-scoped, and matches the constraints it describes — I verified the referenced files (`san-network-attachment.yaml` exists and is unused, `osdMaxUpdatesInParallel: 1` and `upgradeOSDRequiresHealthyPGs: true` are already in place from #639) and the reasoning is internally consistent. No blocking issues found in the diff itself; one operational risk worth confirming before/at apply time is called out below. ## 🟡 Suggestions - **`kubernetes/infrastructure/network-policies/rook-ceph-ns.yaml:9-12,33-35`** — this `CiliumNetworkPolicy` states as a load-bearing assumption that "everything ceph-daemon-y (mons, OSDs, MDSs, RGW, ... mgr, exporter, crashcollector) runs on hostNetwork and is OUTSIDE Cilium endpoint policy entirely." `network.provider: host` is exactly what makes that true today, and this PR sets it to `""`. OSDs restart as part of this same PR (per its own description), so they — and likely the `mgr` deployment (count: 2), which reconciles without a restart gate — will come back on the pod network and become real `CiliumEndpoint`s subject to this policy, not exempt from it. Tracing the existing rules, this probably still works: OSD↔OSD and mgr↔OSD traffic is intra-namespace and covered by the `fromEndpoints: namespace=rook-ceph` rule, and mon-originated traffic (mons stay on hostNetwork per the PR) should still be classified under the `fromEntities: [host, remote-node]` rule regardless of the destination pod's network mode. But I could not confirm Cilium's identity classification for host-originated traffic hitting a pod-network endpoint against a live cluster, and this is the one place where being wrong is a full storage-cluster outage, not a degraded state. Worth an explicit check right after applying (e.g. watch OSD peering/heartbeat in `ceph status`, or `cilium monitor --type drop -n rook-ceph` during/after the OSD restarts) rather than assuming the policy tolerates the transition. Separately, the comment itself is now inaccurate for OSDs/mgr once this lands — worth a follow-up update so it doesn't mislead the next reader into thinking Ceph daemons are still uniformly hostNetwork-exempt. ## Tests No automated test can validate this kind of change — it's a live-cluster behavioral edit that `kustomize-build` in `ci-manifests-test.yml` will only check for YAML/kustomize structural validity, not runtime effect. That's an accurate limitation the PR itself acknowledges (it states the author will confirm the `addressRanges` removal against the live object after applying, since the resource carries no `managedFields`). No gap to flag beyond the NetworkPolicy interaction above, which is also not something CI can catch. ## Recommendation Looks reasonable — no blocking issues; comment above is a pre-apply verification worth doing, not a reason to hold the PR. <!-- pr-review:sha=a6dd1f4be21b7dbb3cd988458ac943ecee961b2b -->
Sign in to join this conversation.