Skip to content

Derive the storage NetworkPolicy node addresses at deploy time #401

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
t0mdavid-m wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Derive the storage NetworkPolicy node addresses at deploy time #401

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d33183b
Derive the storage NetworkPolicy node addresses at deploy time
t0mdavid-m Aug 28, 2026
3cbfe34
Wrap the deploy sequence in one command
t0mdavid-m Aug 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
88 changes: 36 additions & 52 deletions .github/workflows/build-and-test.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -900,32 +900,24 @@ jobs:
# podSelector and no ipBlock. Left alone, every mount on a node not
# running Ganesha hangs, which the worker's DoNotSchedule spread
# guarantees will happen to exactly one replica.
if ! grep -q 'cidr: 192.0.2.0/24' /tmp/storage.yaml; then
echo "::error::k8s/storage/networkpolicy.yaml no longer carries the 192.0.2.0/24 placeholder; this rewrite would silently no-op and every cross-node mount would hang"
exit 1
fi
# Derived, not hardcoded: kind pins only its IPv6 subnet, so the IPv4
# one comes from Docker's default address pool and is not 172.18/16
# by contract.
NODE_CIDR=$(docker network inspect kind \
-f '{{range .IPAM.Config}}{{.Subnet}} {{end}}' \
| tr ' ' '\n' | grep -v ':' | grep -E '^[0-9.]+/[0-9]+$' | head -n1)
if [ -z "$NODE_CIDR" ]; then
echo "::error::could not read an IPv4 subnet from the kind docker network"
docker network inspect kind -f '{{json .IPAM.Config}}'
exit 1
fi
echo "admitting kind nodes on 2049 from $NODE_CIDR"
sed -i "s|cidr: 192.0.2.0/24|cidr: ${NODE_CIDR}|" /tmp/storage.yaml
# kubeconform validated /tmp/storage-rendered.yaml, not the file that
# is actually applied. Re-check the one value the seds above compute
# rather than copy, so a malformed CIDR fails here and names itself
# instead of surfacing as an API-server rejection mid-retry-loop.
if ! grep -Eq '^[[:space:]]*cidr: [0-9]+\.[0-9]+\.[0-9]+\.[0-9]+/[0-9]+$' /tmp/storage.yaml; then
echo "::error::the rewritten NetworkPolicy CIDR is not a bare IPv4 block"
grep -n 'cidr:' /tmp/storage.yaml
exit 1
fi
# The SAME script the deploy documentation tells an operator to run,
# not a kind-only reimplementation of it. It reads node InternalIPs
# from the cluster, emits one /32 each, refuses if any of them falls
# inside the pod network, and refuses on a Cilium whose
# policy-cidr-match-mode would ignore CIDR rules against nodes.
#
# Running it here is the only validation it gets, and it is a real
# one: this executes eight times per run against a two-node cluster,
# and assert_netpol_admits_every_node downstream then proves the
# policy it wrote actually admits every kubelet. The inline version
# this replaces derived the range from `docker network inspect kind`,
# which no production cluster has - so the path an operator actually
# runs was exercised by nothing.
#
# Its own placeholder and CIDR-shape guards replace the ones that
# used to live here; it exits non-zero rather than emitting YAML.
k8s/storage/set-node-cidrs.sh < /tmp/storage.yaml > /tmp/storage-patched.yaml
mv /tmp/storage-patched.yaml /tmp/storage.yaml
applied=0
for i in 1 2 3 4 5; do
if kubectl apply -f /tmp/storage.yaml; then
Expand Down Expand Up @@ -1585,32 +1577,24 @@ jobs:
# podSelector and no ipBlock. Left alone, every mount on a node not
# running Ganesha hangs, which the worker's DoNotSchedule spread
# guarantees will happen to exactly one replica.
if ! grep -q 'cidr: 192.0.2.0/24' /tmp/storage.yaml; then
echo "::error::k8s/storage/networkpolicy.yaml no longer carries the 192.0.2.0/24 placeholder; this rewrite would silently no-op and every cross-node mount would hang"
exit 1
fi
# Derived, not hardcoded: kind pins only its IPv6 subnet, so the IPv4
# one comes from Docker's default address pool and is not 172.18/16
# by contract.
NODE_CIDR=$(docker network inspect kind \
-f '{{range .IPAM.Config}}{{.Subnet}} {{end}}' \
| tr ' ' '\n' | grep -v ':' | grep -E '^[0-9.]+/[0-9]+$' | head -n1)
if [ -z "$NODE_CIDR" ]; then
echo "::error::could not read an IPv4 subnet from the kind docker network"
docker network inspect kind -f '{{json .IPAM.Config}}'
exit 1
fi
echo "admitting kind nodes on 2049 from $NODE_CIDR"
sed -i "s|cidr: 192.0.2.0/24|cidr: ${NODE_CIDR}|" /tmp/storage.yaml
# kubeconform validated /tmp/storage-rendered.yaml, not the file that
# is actually applied. Re-check the one value the seds above compute
# rather than copy, so a malformed CIDR fails here and names itself
# instead of surfacing as an API-server rejection mid-retry-loop.
if ! grep -Eq '^[[:space:]]*cidr: [0-9]+\.[0-9]+\.[0-9]+\.[0-9]+/[0-9]+$' /tmp/storage.yaml; then
echo "::error::the rewritten NetworkPolicy CIDR is not a bare IPv4 block"
grep -n 'cidr:' /tmp/storage.yaml
exit 1
fi
# The SAME script the deploy documentation tells an operator to run,
# not a kind-only reimplementation of it. It reads node InternalIPs
# from the cluster, emits one /32 each, refuses if any of them falls
# inside the pod network, and refuses on a Cilium whose
# policy-cidr-match-mode would ignore CIDR rules against nodes.
#
# Running it here is the only validation it gets, and it is a real
# one: this executes eight times per run against a two-node cluster,
# and assert_netpol_admits_every_node downstream then proves the
# policy it wrote actually admits every kubelet. The inline version
# this replaces derived the range from `docker network inspect kind`,
# which no production cluster has - so the path an operator actually
# runs was exercised by nothing.
#
# Its own placeholder and CIDR-shape guards replace the ones that
# used to live here; it exits non-zero rather than emitting YAML.
k8s/storage/set-node-cidrs.sh < /tmp/storage.yaml > /tmp/storage-patched.yaml
mv /tmp/storage-patched.yaml /tmp/storage.yaml
applied=0
for i in 1 2 3 4 5; do
if kubectl apply -f /tmp/storage.yaml; then
Expand Down
95 changes: 69 additions & 26 deletions docs/a16-storage-runbook.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,32 +135,75 @@ Decision 4 provisions a **fresh** Cinder PVC in `template-app-storage` rather th
migrating the existing one, so the old volume stays intact and untouched
throughout. That is what makes section 5 cheap.

1. Create `template-app-storage`, labelled `pod-security.kubernetes.io/enforce=privileged`.
2. Label `openms` explicitly `enforce=baseline` — do not rely on the absent label.
3. Provision the new Cinder PVC in `template-app-storage`.
4. Deploy Ganesha with `existingClaim`, `Export_Id: 1`, `deviceBasedFsids: false`,
`strategy: Recreate`, 1 replica, and **explicit memory request and limit**.
5. Apply the default-deny ingress plus the two allow rules on 2049: the
pod-label-scoped one admitting `app: template-app` in `openms`, and an
`ipBlock` for the cluster's node addresses. **The second is not optional
and ships as a placeholder.** The provisioner emits in-tree `nfs:` PVs,
which the kubelet mounts from the node's own address in the host network
namespace — that matches no `podSelector`, so with the placeholder left in
place every mount hangs. Read the addresses from `kubectl get nodes -o wide`
and narrow the range as far as it will go; check it does not contain the pod
CIDR, which would hand every pod in the cluster root over every workspace.
6. Create the PVC in `openms` on the new StorageClass, mounted at the unchanged
path `/workspaces-streamlit-template`. It is a NEW claim, `workspaces-nfs-pvc`,
not an edit of `workspaces-pvc`: a bound PVC's spec is immutable apart from
`resources.requests`, so `kubectl apply` would be rejected outright, and the
only way past that is deleting a claim whose `cinder-csi` class reclaims with
`Delete` — destroying the volume section 5 rolls back to.
7. Seed `.demos` via the fixed initContainer above, and create the `.nfs-probe`
sentinel.
8. Repoint the streamlit and rq-worker Deployments at the new claim. Delete the
`nodeselector.yaml` patches; keep the `memory-tier-*` components as resource
patches; set requests == limits.
9. Deploy the storage canary and the sidebar indicator.
**Steps 1–9 below shipped as manifests and are no longer performed by hand.**
They are kept as a description of what the two applies do, and of the reasoning
behind each, because the reasoning is not recoverable from the YAML alone. The
whole cutover is:

```bash
k8s/deploy.sh # --dry-run first if you want to see it render and validate
```

which is, expanded:

```bash
kubectl kustomize --enable-helm k8s/storage/ \
| k8s/storage/set-node-cidrs.sh \
| kubectl apply -f -

kubectl -n template-app-storage rollout status statefulset -l app=nfs-server --timeout=300s
kubectl apply -k k8s/overlays/prod/
```

Storage root first: it publishes the StorageClass the workspaces PVC claims, so
the other order leaves every pod `Pending` on a class that does not exist. The
script confirms which cluster it is pointed at before touching anything, because
both namespaces are named the same on every cluster and nothing in the later
output would tell you it had gone to the wrong one.

1. `template-app-storage`, labelled `pod-security.kubernetes.io/enforce=privileged`
— `k8s/storage/namespace.yaml`.
2. ~~Label `openms` explicitly `enforce=baseline`.~~ **Reversed during
implementation.** `k8s/base/namespace.yaml` sets `warn` and `audit` at
baseline and does **not** set `enforce`. The reasoning is recorded there: the
app's own pods have no `securityContext`, no `hostPath`, no host namespaces
and no added capabilities anywhere in `k8s/base` or the overlay, so enforcing
buys nothing it does not already satisfy, while an enforced level that
tightens under a cluster upgrade can refuse to admit pods. The storage tier,
which does need more, is isolated in its own namespace for exactly this
reason. **A fork that adds a privileged sidecar to `openms` should revisit
this** — the audit trail will show the violation, but nothing will stop it.
3. The new Cinder PVC — `k8s/storage/nfs-backing-pvc.yaml`.
4. Ganesha with `existingClaim`, `Export_Id: 1`, `deviceBasedFsids: false`,
`strategy: Recreate`, 1 replica and explicit memory request and limit —
`k8s/storage/ganesha-values.yaml`. CI asserts all four
(`assert_storage_identity_values`, `assert_fsids_pinned`).
5. The default-deny ingress plus the two allow rules on 2049 —
`k8s/storage/networkpolicy.yaml`. The node rule is **not optional** and ships
as a placeholder: the provisioner emits in-tree `nfs:` PVs which the kubelet
mounts from the node's own address in the host network namespace, matching no
`podSelector`, so with the placeholder left in place every mount hangs.
`set-node-cidrs.sh` in the pipeline above supplies the real addresses — one
`/32` per node — and refuses if any of them falls inside the pod network, or
if the cluster runs a Cilium whose `policy-cidr-match-mode` would ignore the
rule. **Do not hand-edit the CIDR instead**; that puts cluster-specific
configuration into a tracked manifest.
6. The workspaces PVC in `openms` on the new StorageClass, at the unchanged path
`/workspaces-streamlit-template` — `k8s/base/workspace-pvc.yaml`. It is a NEW
claim, `workspaces-nfs-pvc`, not an edit of `workspaces-pvc`: a bound PVC's
spec is immutable apart from `resources.requests`, so `kubectl apply` would be
rejected outright, and the only way past that is deleting a claim whose
`cinder-csi` class reclaims with `Delete` — destroying the volume section 5
rolls back to.
7. `.demos` seeding — now an initContainer running `docker/seed-demos.sh`
(`k8s/base/streamlit-deployment.yaml`), not the inline script in section 2.
The `.nfs-probe` sentinel is created by the worker's readiness probe on first
run (`src/workflow/health.py`).
8. The Deployments already mount the new claim; the `nodeselector.yaml` patches
are deleted and `assert_no_node_pinning_anywhere` is the ratchet that keeps
them deleted. `memory-tier-*` remain as resource patches with
requests == limits.
9. The sidebar indicator reads the heartbeats `probe_storage()` publishes.

---

Expand Down
29 changes: 23 additions & 6 deletions docs/kubernetes-deployment.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -322,22 +322,39 @@ kubectl -n openms rollout restart deployment/<your-app-name>-streamlit

### Step 6 — Deploy

**Set the node CIDR first.** `k8s/storage/networkpolicy.yaml` ships `192.0.2.0/24` (RFC 5737 TEST-NET-1) as a placeholder, which admits nothing. The provisioner emits in-tree `nfs:` volumes, and the kubelet mounts those from the node's own address rather than from a pod IP, so without this every mount hangs on a CNI that enforces NetworkPolicy. Read the node addresses off the cluster and narrow the range as far as it will go:

```bash
kubectl get nodes -o wide # the INTERNAL-IP column
k8s/deploy.sh # or --dry-run to render and validate, applying nothing
```

Then apply the storage tier **before** the overlay. It publishes the StorageClass the workspaces PVC claims; applied in the other order, every pod sits `Pending` on a class that does not exist yet:
That is the whole deploy. It prints the context and cluster it is about to touch and asks for confirmation first (`--yes` to skip, required when there is no terminal), then does the three things below in the one order that works, waiting between them.

**There is no single `kubectl apply -k` for this, and the reasons are worth knowing** — each is a silent, expensive failure if you do it by hand and get it wrong:

1. **Two roots, in order.** `k8s/storage/` publishes the StorageClass `k8s/base/workspace-pvc.yaml` claims. The other order leaves every pod `Pending` on a class that does not exist, and the message says nothing about ordering. They cannot be merged into one root either — see the top of `k8s/storage/kustomization.yaml`, which is about the namespace transformer clobbering per-object namespaces.
2. **`kubectl apply -k` has no `--enable-helm`.** The storage root inflates the Ganesha chart, so it must be rendered and piped.
3. **The node addresses are not in the repo.** See below.

Done by hand, it is:

```bash
kubectl kustomize --enable-helm k8s/storage/ | kubectl apply -f -
kubectl kustomize --enable-helm k8s/storage/ \
| k8s/storage/set-node-cidrs.sh \
| kubectl apply -f -

kubectl -n template-app-storage rollout status statefulset -l app=nfs-server --timeout=300s

kubectl apply -k k8s/overlays/prod/
```

The first of those needs Helm on `PATH`. Both applies are idempotent, and on an upgrade the storage one is usually a no-op.
**Do not skip `set-node-cidrs.sh`, and do not edit the CIDR by hand instead.** `k8s/storage/networkpolicy.yaml` ships `192.0.2.0/24` (RFC 5737 TEST-NET-1) as a placeholder, which admits nothing. The provisioner emits in-tree `nfs:` volumes and the kubelet mounts those from the node's own address rather than from a pod IP — matching no `podSelector` — so on a CNI that enforces NetworkPolicy, an `ipBlock` covering the nodes is the only thing that admits the mount. Left unset, every mount hangs with `mount.nfs: Connection timed out`, which names nothing.

The script reads the node addresses from the cluster you are pointed at and patches the **rendered stream** — it never edits a tracked file, so your fork does not diverge from upstream on a cluster-specific line. It emits one `/32` per node, which is tighter than a range chosen by hand, and it refuses rather than proceeding when:

- a node address falls inside the pod network (the export is `no_root_squash`, so that would give every pod in the cluster root over every workspace);
- it cannot evaluate that overlap at all;
- the cluster runs **Cilium** with `policy-cidr-match-mode` unset. From Cilium 1.14 remote nodes carry the `remote-node` identity and CIDR rules do not select node identities without that flag — so the policy would be correct and silently ignored, producing exactly the same hang. Fix the cluster, or override with `ALLOW_CILIUM_WITHOUT_NODE_CIDR_MATCH=1` if you know it admits node traffic another way. **Do not widen the CIDR to work around it.**

It needs `kubectl`, `yq` and `python3` on `PATH`; the first apply also needs Helm. Both applies are idempotent, and on an upgrade the storage one is usually a no-op.

### Step 7 — Verify

Expand Down
Loading
Loading