Skip to content

Commit 58ccb48

Browse files
t0mdavid-mclaude
andcommitted
Let the scheduler place worker pods
Removes per-node memory tiering entirely. Nothing in k8s/ now pins a pod to a node — no nodeSelector, no nodeName, no nodeAffinity, no openms.de/memory-tier labels. A tier survives only as a pod size, which is a request the scheduler can act on rather than a label an operator has to maintain. The memory-tier components were already the right shape; they just carried a nodeselector.yaml patch targeting `kind: Deployment` with no name, which landed on redis and streamlit as well as the worker while missing the CronJob entirely. Deleting the patch files and their kustomization entries is atomic: a kustomization referencing a deleted path fails to build. requests now equal limits in both tiers. That is architectural rather than tidy: the pod size is the tier, so a worker requesting 1Gi against a 16Gi limit would be schedulable anywhere and then OOM. Guaranteed QoS also moves oom_score_adj from roughly 937 to -997, so the worker stops being near the top of the node's kill list. rq-worker moves to a fixed replica count with topologySpreadConstraints over kubernetes.io/hostname. Those constraints were inert before: maxSkew is measured over eligible domains, and the nodeSelector left exactly one node eligible, so any spread rule written earlier would have passed while changing nothing. k8s/overlays/ci exists because a production-sized worker cannot be scheduled on a CI runner; it is prod with the worker shrunk, so the kind jobs exercise the real manifests rather than a hand-maintained copy. docs/kubernetes-deployment.md claimed the VolumeBinding plugin pins pods to the node holding the attached volume. It does not — the scheduler checks PV node affinity only, and the result is a Multi-Attach hang at attach time rather than a scheduling refusal. That belief is why the old topology looked intentional. configure-k8s-deployment.md had no question about worker size or replica count at all, and told operators co-location was enforced by the RWO mount. Both are now wrong and both are fixed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XFgL1SSeMCM3J1ZuAVTXv7
1 parent fe6f012 commit 58ccb48

13 files changed

Lines changed: 731 additions & 190 deletions

.claude/skills/configure-k8s-deployment.md

Lines changed: 61 additions & 23 deletions
Large diffs are not rendered by default.

.github/kind-config.yaml

Lines changed: 20 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,27 +1,35 @@
11
kind: Cluster
22
apiVersion: kind.x-k8s.io/v1alpha4
3-
# Two-node cluster that mirrors the production memory-tier topology, so
4-
# the Build-and-Test job passes regardless of which tier a fork selects
5-
# in its overlay (memory-tier-low or memory-tier-high). Without both
6-
# labels present, flipping the overlay would leave pods Pending on the
7-
# single kind node.
3+
# Two schedulable nodes, which is the smallest cluster that can observe the
4+
# property the deployment now rests on: workers spread across hosts, and one
5+
# workspace volume reachable from all of them. On a single node every spread
6+
# constraint is satisfied vacuously - maxSkew is measured over eligible
7+
# domains and one domain is always perfectly balanced - so a one-node cluster
8+
# would report success for exactly the arrangement this suite exists to rule
9+
# out.
10+
#
11+
# No node labels. Nothing under k8s/ selects a node any more: the scheduler
12+
# places pods, and the manifests only declare how big a worker is. The nodes
13+
# carried openms.de/memory-tier=low and =high while the memory-tier components
14+
# patched a matching nodeSelector onto every Deployment; those patches are
15+
# gone, and leaving the labels behind would imply the pinning had survived
16+
# somewhere. assert_no_node_pinning_anywhere text-scans this file for exactly
17+
# that.
818
nodes:
919
- role: control-plane
1020
# Multi-node kind clusters taint the control-plane with
11-
# node-role.kubernetes.io/control-plane:NoSchedule. Clear it so
12-
# app pods with nodeSelector memory-tier=low can actually land
13-
# here (single-node kind had no such taint, hence the original
14-
# workflow passed without this patch).
21+
# node-role.kubernetes.io/control-plane:NoSchedule. Clear it so app pods
22+
# can land here as well: with the taint in place there is one schedulable
23+
# node, and every cross-node assertion - two pods on two nodes, the
24+
# cross-node write, the cross-node flock, the worker spread - collapses
25+
# into a tautology it cannot fail.
1526
kubeadmConfigPatches:
1627
- |
1728
kind: InitConfiguration
1829
nodeRegistration:
1930
taints: []
2031
labels:
21-
openms.de/memory-tier: low
2232
# ingress-ready is required by the kind variant of the
2333
# ingress-nginx deploy manifest applied in CI.
2434
ingress-ready: "true"
2535
- role: worker
26-
labels:
27-
openms.de/memory-tier: high

CLAUDE.md

Lines changed: 184 additions & 110 deletions
Large diffs are not rendered by default.

docs/kubernetes-deployment.md

Lines changed: 123 additions & 25 deletions
Large diffs are not rendered by default.

k8s/base/rq-worker-deployment.yaml

Lines changed: 186 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,60 @@ metadata:
55
labels:
66
component: rq-worker
77
spec:
8-
replicas: 1
8+
# Two workers, not one. RQ runs one job per worker process, so `replicas: 1`
9+
# serialised every workflow in a deployment however much hardware sat
10+
# underneath it - and on the de.NBI pair the second node could not have been
11+
# used anyway, because the memory-tier component pinned every Deployment to
12+
# one labelled node and the workspace claim was ReadWriteOnce on a Cinder
13+
# volume that attaches to exactly one node. Both of those are gone. This is
14+
# the line that actually spends the second node.
15+
#
16+
# Fixed rather than autoscaled: each worker is Guaranteed QoS at whichever
17+
# size k8s/components/memory-tier-*/worker-resources.yaml gives it, so N
18+
# replicas reserve N times that size for as long as they run. The count is a
19+
# capacity decision against real nodes - see the worker-size and
20+
# worker-replica questions in .claude/skills/configure-k8s-deployment.md.
21+
#
22+
# 2 is one per node on the production pair, which is the point at which the
23+
# spread constraint below has anything to do. A higher count is legal
24+
# (maxSkew 1 permits 2/1 across two nodes) but only pays off where a node can
25+
# hold two workers of this size at once. It is also the first configuration
26+
# in which two workflow runs are genuinely concurrent: everything they share
27+
# goes through the workspace volume, so a fork that adds shared mutable state
28+
# outside it has no serialisation left to hide behind.
29+
replicas: 2
30+
# Explicit, because the default is wrong here in a way that only shows up
31+
# once replicas > 1. RollingUpdate defaults to 25%/25%, which against 2
32+
# replicas rounds to maxSurge 1 and maxUnavailable **0** (maxUnavailable
33+
# rounds down, and the 0/0 fallback does not apply because surge is 1). A
34+
# Deployment's Available condition needs `replicas - maxUnavailable` ready,
35+
# so with 0 the whole Deployment goes non-Available the moment either worker
36+
# is Pending or NotReady - and both of those are routine here rather than
37+
# exceptional:
38+
# - whenUnsatisfiable: DoNotSchedule below parks a replica in Pending
39+
# during any node drain, since a cordoned node still counts as a domain
40+
# under the default nodeTaintsPolicy;
41+
# - the readinessProbe below fails every worker whose mount is blocked
42+
# during a routine Ganesha restart.
43+
# Either would then hang `kubectl rollout status deployment/<slug>-rq-worker`
44+
# and fail CI's `kubectl wait --for=condition=available`, turning a
45+
# degradation that the design calls survivable into a blocked rollout.
46+
#
47+
# maxUnavailable 1: one worker down is a queue running at half rate, which is
48+
# what "visible and recoverable" has to mean if it is to mean anything. That
49+
# the OTHER replica is up is not weakened by this - it is asserted directly,
50+
# against .spec.replicas, by assert_workers_spread_across_nodes.
51+
#
52+
# maxSurge 0: a surge replica needs a whole extra worker's worth of memory
53+
# free somewhere, and these are Guaranteed pods sized to most of a node
54+
# (180Gi in the high tier). Surging first would leave the new pod Pending on
55+
# a cluster that has exactly enough room for the steady-state count, which is
56+
# the normal case. Terminate-then-replace instead.
57+
strategy:
58+
type: RollingUpdate
59+
rollingUpdate:
60+
maxSurge: 0
61+
maxUnavailable: 1
962
selector:
1063
matchLabels:
1164
component: rq-worker
@@ -14,6 +67,50 @@ spec:
1467
labels:
1568
component: rq-worker
1669
spec:
70+
# Spread the workers over nodes: one each until every node has one.
71+
#
72+
# maxSkew is measured over ELIGIBLE domains, which is why this had to
73+
# wait for the nodeSelector deletion rather than shipping alongside it.
74+
# With a selector matching exactly one node there is one eligible domain,
75+
# every distribution has skew 0, and a constraint written here would have
76+
# passed while changing nothing at all.
77+
#
78+
# DoNotSchedule, not ScheduleAnyway: soft spreading is a scoring
79+
# preference the scheduler can trade away against other priorities, and a
80+
# worker that usually spreads is a second node that is used right up
81+
# until the day it matters.
82+
#
83+
# The cost is a Pending replica during node maintenance: with the default
84+
# nodeTaintsPolicy a cordoned node still counts as a domain, so a drain
85+
# parks the evicted worker in Pending rather than doubling it up on the
86+
# survivor. That is visible in `kubectl get pods` and it clears when the
87+
# node comes back - but only because `strategy.maxUnavailable` above is
88+
# 1. At the default of 0 for this replica count the same Pending replica
89+
# would take the Deployment out of Available and hang every subsequent
90+
# `kubectl rollout status`, which is a blocked rollout, not a
91+
# degradation. The two settings are a pair; changing one without the
92+
# other is what makes this bite. Add `nodeTaintsPolicy: Honor` here
93+
# (Kubernetes >= 1.27) if node maintenance becomes routine enough that
94+
# even the Pending replica is unwelcome.
95+
#
96+
# The selector names `component` alone. The overlay's
97+
# `commonLabels: {app: <slug>}` is copied into it by kustomize, whose
98+
# commonLabels field specs reach
99+
# topologySpreadConstraints/labelSelector/matchLabels with create: false
100+
# (verified against the kustomize 5.7.1 built into kubectl). That scoping
101+
# is load-bearing: every fork of this template deploys into the same
102+
# `openms` namespace and topology spread counts pods per namespace, so
103+
# without the app label one fork's workers would be counted against
104+
# another's skew. A fork that drops commonLabels from its overlay loses
105+
# that scoping, and the constraint starts balancing every rq-worker in
106+
# the namespace as though they were all its own.
107+
topologySpreadConstraints:
108+
- maxSkew: 1
109+
topologyKey: kubernetes.io/hostname
110+
whenUnsatisfiable: DoNotSchedule
111+
labelSelector:
112+
matchLabels:
113+
component: rq-worker
17114
containers:
18115
- name: rq-worker
19116
image: openms-streamlit
@@ -30,6 +127,93 @@ spec:
30127
value: "redis://redis:6379/0"
31128
- name: WORKSPACES_DIR
32129
value: "/workspaces-streamlit-template"
130+
# Downward API. The storage heartbeat is keyed per node
131+
# (storage:ok:<node>) and the node registration alongside it
132+
# (storage:node:<node>) is what makes a *missing* heartbeat
133+
# readable, so a healthy node cannot mask a node whose mount has
134+
# wedged - with one shared key the indicator would stay green
135+
# straight through a real outage. Without this env var
136+
# health.current_node_name() falls back to the pod hostname, which
137+
# changes on every restart and keys the heartbeat per pod instead
138+
# of per node.
139+
- name: NODE_NAME
140+
valueFrom:
141+
fieldRef:
142+
fieldPath: spec.nodeName
143+
# Readiness only, and deliberately no liveness counterpart: a
144+
# liveness failure would SIGKILL the container mid-TOPP-job, and a
145+
# restart cannot fix a wedged NFS mount - it would destroy hours of
146+
# work while the fault persists. rq-worker sits behind no Service, so
147+
# readiness has no traffic effect either way; it is purely an
148+
# alertable signal, and it doubles as the writer of the sidebar's
149+
# storage heartbeat (src/workflow/health.py).
150+
#
151+
# One process, not a shell pipeline, because the ordering inside it
152+
# is the contract and belongs somewhere a test can drive it
153+
# (tests/test_storage_health.py). `probe_storage()` registers the
154+
# node, *then* writes to the volume, and publishes the heartbeat only
155+
# if that write succeeded. Nothing here is wrapped in `|| true`: a
156+
# `|| true` on the storage check would report Ready on a wedged mount
157+
# and publish a green heartbeat with it, which is the whole failure
158+
# this probe exists to catch.
159+
#
160+
# A write, not a `stat`: with default `actimeo` a stat can be answered
161+
# from the client's attribute cache while the server is gone, which
162+
# is a false green. A write always reaches the server, and it creates
163+
# the sentinel on a fresh volume instead of failing forever on a file
164+
# nothing else makes. The file is dot-named and not a directory, so
165+
# clean-up-workspaces.py skips it twice over.
166+
#
167+
# The interpreter is named explicitly rather than activated: `conda
168+
# activate` costs a few hundred ms of shell hooks on every probe, and
169+
# this is the same env the container command activates one screen up.
170+
#
171+
# Timing, against the ~90s NFSv4 grace period a Ganesha restart
172+
# imposes - all I/O blocks until it ends, so the numbers have to
173+
# outlast it or every routine restart flaps the worker:
174+
# inner timeout 15s - a cold interpreter plus `import redis` is
175+
# ~200ms idle, but this pod is by design
176+
# CPU-saturated running TOPP tools under a CFS
177+
# quota, and the two Redis calls are bounded at
178+
# health.REDIS_SOCKET_TIMEOUT (2s) each. The
179+
# budget has to cover all of that *and* leave
180+
# room for a slow-but-alive mount, or the probe
181+
# reports a storage fault that is really a busy
182+
# node.
183+
# timeoutSeconds 20 - kubelet's own bound, above the inner one so
184+
# that a reachable-but-slow mount produces a
185+
# clean exit 1 from `timeout`. On a genuinely
186+
# wedged `hard` mount it is kubelet that ends
187+
# the attempt, not `timeout`: the blocked write
188+
# sits in uninterruptible sleep, TERM and KILL
189+
# are both ignored, and `timeout` then blocks in
190+
# wait() for a child that cannot die. The probe
191+
# still fails, and the process stays until the
192+
# mount recovers.
193+
# periodSeconds 30 - one attempt per 30s. Those unkillable
194+
# processes accumulate one per period during an
195+
# outage, so the cadence bounds the pile.
196+
# failureThreshold 5 - 4 x 30s + up to 20s = ~140s of continuous
197+
# failure before NotReady, ~50s clear of the
198+
# grace period.
199+
# health.STORAGE_HEARTBEAT_TTL (120s) is 4 x periodSeconds, so the
200+
# sidebar indicator goes red after three missed refreshes - still
201+
# before the worker is marked NotReady. Degradation should be visible,
202+
# not fatal.
203+
readinessProbe:
204+
exec:
205+
command:
206+
- /bin/bash
207+
- -c
208+
- |
209+
set -u
210+
py=/root/miniforge3/envs/streamlit-env/bin/python
211+
exec timeout -k 3 15 "$py" -m src.workflow.health --probe
212+
initialDelaySeconds: 15
213+
periodSeconds: 30
214+
timeoutSeconds: 20
215+
failureThreshold: 5
216+
successThreshold: 1
33217
volumeMounts:
34218
- name: workspaces
35219
mountPath: /workspaces-streamlit-template
@@ -40,7 +224,7 @@ spec:
40224
volumes:
41225
- name: workspaces
42226
persistentVolumeClaim:
43-
claimName: workspaces-pvc
227+
claimName: workspaces-nfs-pvc
44228
- name: config
45229
configMap:
46230
name: streamlit-config

k8s/components/memory-tier-high/kustomization.yaml

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,16 @@
11
apiVersion: kustomize.config.k8s.io/v1alpha1
22
kind: Component
33

4+
# Resource patches only. A tier is a POD SIZE, not a place: nothing in this
5+
# component - and nothing anywhere else in k8s/ - decides which node a pod
6+
# lands on. That is the scheduler's job, and the pinning patch that used to
7+
# live here took it away, unscoped, from every Deployment in the base
8+
# (streamlit and redis included), which is what made the second node
9+
# unusable. It also made topologySpreadConstraints inert, since maxSkew is
10+
# measured over eligible domains and exactly one node was ever eligible.
11+
# `assert_no_node_pinning_anywhere` in .github/scripts/ci-assertions.sh keeps
12+
# it from coming back on a fork rebase.
413
patches:
5-
- path: nodeselector.yaml
6-
target:
7-
kind: Deployment
814
- path: streamlit-resources.yaml
915
target:
1016
kind: Deployment

k8s/components/memory-tier-high/nodeselector.yaml

Lines changed: 0 additions & 4 deletions
This file was deleted.

k8s/components/memory-tier-high/worker-resources.yaml

Lines changed: 48 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,60 @@ kind: Deployment
33
metadata:
44
name: rq-worker
55
spec:
6+
# The replica count travels with the size, and only this tier overrides it.
7+
# k8s/base/rq-worker-deployment.yaml says 2, which is one per node on a pair
8+
# of equally-sized nodes - true of the low tier, false here. A 180Gi
9+
# Guaranteed request fits on a high-memory node and nowhere else, and the
10+
# de.NBI pair has one of those, so an inherited `replicas: 2` puts the second
11+
# worker in Pending permanently: `whenUnsatisfiable: DoNotSchedule` forbids
12+
# doubling it up on the node that could hold it, and no other node can.
13+
#
14+
# Nothing in kustomize couples a resource patch to a replica count, so the
15+
# coupling has to be written down somewhere; here is the only place that sees
16+
# both numbers. A fork with N high-memory nodes raises this to N. Note that
17+
# at 1 replica the spread constraint in the base has nothing to do and
18+
# assert_workers_spread_across_nodes will say so - that assertion is written
19+
# for the shipped (low-tier) sizing that CI deploys, and a high-tier fork
20+
# running it needs N >= 2 for it to mean anything.
21+
replicas: 1
622
template:
723
spec:
824
containers:
925
- name: rq-worker
26+
# requests == limits for Guaranteed QoS; see
27+
# ../memory-tier-low/worker-resources.yaml for why that is
28+
# architectural rather than advisory once the tier is a pod size
29+
# instead of a node label.
30+
#
31+
# Specific to this tier is what the number now means. 180Gi was
32+
# written as a burst ceiling - uniform across heavy workers so that
33+
# whichever app was active could borrow the shared pool - and as a
34+
# request it stops being a borrow and becomes a reservation the
35+
# scheduler holds whether a workflow is running or not, i.e. most of
36+
# a high-memory node. Fixed workers statically partition capacity;
37+
# that is the accepted trade at this cluster size (decision 10 in
38+
# node-distributed-denbi/A16-DECISIONS.md), but a fork sharing its
39+
# node pool with another heavy app should size the worker to its own
40+
# slice and lower BOTH blocks together rather than reopening a gap
41+
# between them.
42+
#
43+
# cpu 20 sits exactly ON the LimitRange maximum in
44+
# k8s/base/limitrange.yaml (200Gi / 20 cpu), so it is a ceiling as
45+
# well as a reservation: this number can only move down unless the
46+
# LimitRange moves first. Raising it alone does not produce a
47+
# Pending pod that says "no node has this much CPU" - the API server
48+
# rejects the Deployment's pods outright as a LimitRange violation,
49+
# which reads as a quota error rather than a capacity one.
50+
#
51+
# Down is the direction worth going anyway. The worker cannot spend
52+
# 20: run_topp parallelises across files up to max_threads (2 in
53+
# online deployments) and TOPPBase caps each tool through
54+
# omp_set_num_threads, so bringing cpu down toward max_threads is
55+
# the first thing worth tuning here - keeping requests == limits.
1056
resources:
1157
requests:
12-
memory: "2Gi"
13-
cpu: "2"
58+
memory: "180Gi"
59+
cpu: "20"
1460
limits:
1561
memory: "180Gi"
1662
cpu: "20"

k8s/components/memory-tier-low/kustomization.yaml

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,16 @@
11
apiVersion: kustomize.config.k8s.io/v1alpha1
22
kind: Component
33

4+
# Resource patches only. A tier is a POD SIZE, not a place: nothing in this
5+
# component - and nothing anywhere else in k8s/ - decides which node a pod
6+
# lands on. That is the scheduler's job, and the pinning patch that used to
7+
# live here took it away, unscoped, from every Deployment in the base
8+
# (streamlit and redis included), which is what made the second node
9+
# unusable. It also made topologySpreadConstraints inert, since maxSkew is
10+
# measured over eligible domains and exactly one node was ever eligible.
11+
# `assert_no_node_pinning_anywhere` in .github/scripts/ci-assertions.sh keeps
12+
# it from coming back on a fork rebase.
413
patches:
5-
- path: nodeselector.yaml
6-
target:
7-
kind: Deployment
814
- path: streamlit-resources.yaml
915
target:
1016
kind: Deployment

k8s/components/memory-tier-low/nodeselector.yaml

Lines changed: 0 additions & 4 deletions
This file was deleted.

0 commit comments

Comments
 (0)