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.
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`.
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>
🤖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 -->
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Intermediate state, not a destination.
san-network-attachment.yamlis already in the cluster and stays unused until phase B setsprovider: multus.The detour is forced, not chosen. Rook's CRD rejects the direct transition:
host→multusis refused outright; it has to pass through empty. An empty provider means the default CNI, andaddressRangesonly takes effect underprovider: 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:
The removal of
addressRangesdoes not appear there: this resource carries nomanagedFields, so a client-side diff cannot see a field deletion. Inert either way — Rook does not readaddressRangeswithoutprovider: 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 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>🤖 Automated review — model
CLI default(complexity: complex), commita6dd1f4be21b7dbb3cd988458ac943ecee961b2b.Summary
PR #640 is phase A of a two-phase migration of the Ceph cluster network off
provider: hostand onto Multus. It changeskubernetes/infrastructure/rook-ceph-cluster/cephcluster.yamlto setnetwork.provider: ""(fromhost) and drops the now-inertaddressRangesblock, forced by Rook's CRD validation rule that rejects a directhost→multustransition. The change itself is small, well-scoped, and matches the constraints it describes — I verified the referenced files (san-network-attachment.yamlexists and is unused,osdMaxUpdatesInParallel: 1andupgradeOSDRequiresHealthyPGs: trueare 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— thisCiliumNetworkPolicystates 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: hostis 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 themgrdeployment (count: 2), which reconciles without a restart gate — will come back on the pod network and become realCiliumEndpoints 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-cephrule, and mon-originated traffic (mons stay on hostNetwork per the PR) should still be classified under thefromEntities: [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 inceph status, orcilium monitor --type drop -n rook-cephduring/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-buildinci-manifests-test.ymlwill 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 theaddressRangesremoval against the live object after applying, since the resource carries nomanagedFields). 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.