Skip to content

Commit 525e416

Browse files
fix(kubernetes-nodes): let ComputePlane pools set maxUnavailable and bound it to int32
The ComputePlane wrapper forwards only the pool fields it names, and maxUnavailable was not one of them. A ComputePlane pool without room for one extra VM therefore had no way back to the drain-first rollout, and its rollouts stalled. CAPI decodes maxUnavailable as an int32, so a value above 2147483647 passed the chart schema and was then refused by the MachineDeployment decoder. Assisted-by: LLM Signed-off-by: Mattia Eleuteri <[email protected]>
1 parent ed40d83 commit 525e416

11 files changed

Lines changed: 71 additions & 3 deletions

File tree

‎api/apps/v1alpha1/kubernetesnodes/types.go‎

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎packages/apps/kubernetes-nodes/tests/rollout_strategy_test.yaml‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,3 +114,23 @@ tests:
114114
maxUnavailable: -1
115115
asserts:
116116
- failedTemplate: {}
117+
118+
# CAPI decodes the field as an int32, so a larger value would render and
119+
# then be refused by the MachineDeployment decoder.
120+
- it: refuses a maxUnavailable above the int32 range (schema maximum)
121+
set:
122+
maxUnavailable: 2147483648
123+
asserts:
124+
- failedTemplate:
125+
errorPattern: "maxUnavailable"
126+
127+
- it: accepts the int32 maximum
128+
set:
129+
maxUnavailable: 2147483647
130+
documentSelector:
131+
path: kind
132+
value: MachineDeployment
133+
asserts:
134+
- equal:
135+
path: spec.strategy.rollingUpdate.maxUnavailable
136+
value: 2147483647

‎packages/apps/kubernetes-nodes/values.schema.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,7 @@
149149
"maxUnavailable": {
150150
"description": "Workers a rollout (instance type, image or version change) may remove before their replacement is up. Unset, it is 0, so each replacement joins before an old worker is drained, and 1 on a pool with `gpus`, whose replacement may need the very GPU the old worker holds. With 0, a rollout needs room for one extra VM: a pool short of quota or capacity for it stalls, with the new Machine Pending, until room is made or `maxUnavailable` is set to 1. Set 1 on a Proxmox pool whose template passes a GPU through: the chart cannot see that GPU, and with 0 the rollout would wait for it forever.",
151151
"type": "integer",
152+
"maximum": 2147483647,
152153
"minimum": 0
153154
},
154155
"kernelModules": {

‎packages/apps/kubernetes-nodes/values.yaml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@ gpus: []
7979

8080
## @param {*int} [maxUnavailable] - Workers a rollout (instance type, image or version change) may remove before their replacement is up. Unset, it is 0, so each replacement joins before an old worker is drained, and 1 on a pool with `gpus`, whose replacement may need the very GPU the old worker holds. With 0, a rollout needs room for one extra VM: a pool short of quota or capacity for it stalls, with the new Machine Pending, until room is made or `maxUnavailable` is set to 1. Set 1 on a Proxmox pool whose template passes a GPU through: the chart cannot see that GPU, and with 0 the rollout would wait for it forever.
8181
## @minimum 0
82+
## @maximum 2147483647
8283

8384
## @param {[]KernelModule} [kernelModules] - Kernel modules loaded on every worker in this pool, emitted as Talos `machine.kernel.modules`. A Talos system extension installs a module but does not load it, so an extension-provided driver needs its modules declared here. Leave unset to let the chart decide: a pool holding at least one `nvidia.com/*` GPU gets `nvidia` (with `NVreg_NvLinkDisable=1`, which the gpu-operator driver container applies on non-Talos workers; on Talos that container has to be turned off or it clashes with the system extension, and nothing then mounts the ConfigMap carrying the parameter), `nvidia_uvm`, `nvidia_drm`, `nvidia_modeset` (that order — Talos loads the list in sequence and the last three depend on the first), and any other pool gets nothing. Set a non-empty list to replace the chart's choice entirely, or `[]` to opt out and emit no modules even on a GPU pool. The field carries no default, and unset is not the same value as `[]`: a generated parameter table that shows `[]` in its Default column is printing what it prints for any array with no default, not the effective value — leaving the field out selects the automatic set, writing `[]` disables it. The module still has to be in the image, and which extensions the image carries is the pool's schematic: a GPU pool points `osImage.factory.schematicID` (or `osImage.builtin.schematicID`) at a schematic with the NVIDIA extensions, and on Blackwell (GB202) that must be the open-kernel-modules extension. The automatic set keys on the `nvidia.com/` resource prefix alone and nothing cross-checks it against that schematic; on Talos v1.13 a declared module the image does not ship leaves its controller retrying rather than failing the boot, so the symptom is a missing driver, not a dead node. This value is part of the worker machine config, whose content hash names the `TalosConfigTemplate`, so changing it rolls every existing worker in the pool rather than only affecting nodes created later. The automatic set is itself such a change, on the upgrade that introduces this field, for every existing pool that holds an `nvidia.com/*` GPU and leaves the field unset: that pool rolls.
8485

‎packages/extra/computeplane/README.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,4 +59,5 @@ See the full design in
5959
| `nodeGroups[name].kubelet.kubeReservedCpu` | CPU reserved for kubelet and container runtime. Auto-computed from instanceType if empty. | `string` | `""` |
6060
| `nodeGroups[name].kubelet.evictionHardMemory` | Hard eviction threshold for memory (absolute like 200Mi or percentage like 7%). | `string` | `7%` |
6161
| `nodeGroups[name].kubelet.evictionSoftMemory` | Soft eviction threshold for memory (absolute like 1Gi or percentage like 10%). | `string` | `10%` |
62+
| `nodeGroups[name].maxUnavailable` | Workers a rollout may remove before their replacement is up. Unset, the `kubernetes-nodes` pool chart uses 0, so each replacement joins before an old worker is drained (1 on a pool with `gpus`). Set 1 on a pool without room for one extra VM, whose rollout would otherwise stall with the new Machine Pending. | `*int` | `null` |
6263

‎packages/extra/computeplane/templates/cluster.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -211,7 +211,7 @@ spec:
211211
clone or repoint itself at another Image Factory, both of which are
212212
operator decisions about the platform's own images. Keep this list in
213213
sync with the kubernetes-nodes values schema's pool-shape params. */}}
214-
{{- with pick $group "minReplicas" "maxReplicas" "instanceType" "diskSize" "storageClass" "roles" "resources" "gpus" "kubelet" "logSerialConsole" "maxUnhealthy" "nodeStartupTimeout" "readyUnknownTimeout" "readyFalseTimeout" "podCpuLimit" "podCpuRequest" "maxNodeProvisionTime" }}
214+
{{- with pick $group "minReplicas" "maxReplicas" "instanceType" "diskSize" "storageClass" "roles" "resources" "gpus" "kubelet" "logSerialConsole" "maxUnhealthy" "nodeStartupTimeout" "readyUnknownTimeout" "readyFalseTimeout" "podCpuLimit" "podCpuRequest" "maxNodeProvisionTime" "maxUnavailable" }}
215215
{{- toYaml . | nindent 4 }}
216216
{{- end }}
217217
valuesFrom:

‎packages/extra/computeplane/tests/cluster_test.yaml‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -280,6 +280,41 @@ tests:
280280
- notExists:
281281
path: spec.values.nameOverride
282282

283+
# kubernetes-nodes drains a worker only once its replacement is up; a pool
284+
# without room for one extra VM needs maxUnavailable 1 to finish a rollout,
285+
# so the allowlist must forward it.
286+
- it: forwards maxUnavailable to the pool chart
287+
documentSelector:
288+
path: metadata.name
289+
value: kubernetes-nodes-computeplane-cluster-md0
290+
set:
291+
nodeGroups:
292+
md0:
293+
minReplicas: 1
294+
maxReplicas: 3
295+
instanceType: u1.large
296+
diskSize: 32Gi
297+
roles: ["ingress-nginx"]
298+
maxUnavailable: 1
299+
asserts:
300+
- equal:
301+
path: spec.values.maxUnavailable
302+
value: 1
303+
304+
- it: refuses a maxUnavailable above the int32 range CAPI decodes
305+
set:
306+
nodeGroups:
307+
md0:
308+
minReplicas: 1
309+
maxReplicas: 3
310+
instanceType: u1.large
311+
diskSize: 32Gi
312+
roles: ["ingress-nginx"]
313+
maxUnavailable: 2147483648
314+
asserts:
315+
- failedTemplate:
316+
errorPattern: "maxUnavailable"
317+
283318
# Helm caps a HelmRelease name at 53 chars and each pool renders a 38-char fixed
284319
# prefix, so a pool key over 15 chars would create a release the apiserver
285320
# accepts but Flux never reconciles. The render must refuse instead (the same

‎packages/extra/computeplane/values.schema.json‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,12 @@
9393
"type": "integer",
9494
"default": 10
9595
},
96+
"maxUnavailable": {
97+
"description": "Workers a rollout may remove before their replacement is up. Unset, the `kubernetes-nodes` pool chart uses 0, so each replacement joins before an old worker is drained (1 on a pool with `gpus`). Set 1 on a pool without room for one extra VM, whose rollout would otherwise stall with the new Machine Pending.",
98+
"type": "integer",
99+
"maximum": 2147483647,
100+
"minimum": 0
101+
},
96102
"minReplicas": {
97103
"description": "Minimum number of replicas.",
98104
"type": "integer",

‎packages/extra/computeplane/values.yaml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,9 @@
3030
## @field {Resources} [resources] - Explicit CPU and memory for each worker node, as an alternative to `instanceType` sizing. Set both `cpu` and `memory` together or neither.
3131
## @field {[]GPU} gpus - List of GPUs to attach (NVIDIA driver requires at least 4 GiB RAM).
3232
## @field {Kubelet} [kubelet] - Kubelet resource reservations for this node group.
33+
## @field {*int} [maxUnavailable] - Workers a rollout may remove before their replacement is up. Unset, the `kubernetes-nodes` pool chart uses 0, so each replacement joins before an old worker is drained (1 on a pool with `gpus`). Set 1 on a pool without room for one extra VM, whose rollout would otherwise stall with the new Machine Pending.
34+
## @minimum 0
35+
## @maximum 2147483647
3336

3437
## @param {map[string]NodeGroup} nodeGroups - Worker node groups of the ComputePlane cluster (cluster shape only — the security posture is fixed by the module and cannot be overridden through this knob: the module forwards only the documented per-pool fields to the `kubernetes-nodes` pool chart, so any other `NodeGroup` field, e.g. `talos`/`version`/`images`, is silently dropped — accepted but inert). Same structure as the `kubernetes` application's `nodeGroups`. When left empty, the module itself materialises a default scale-from-zero `md0` pool with `roles: [ingress-nginx]` (the wrapped `kubernetes` chart no longer injects one after the Phase 2 split), so the cluster sits idle until a pending Pod (e.g. the ingress-nginx controller) triggers the cluster-autoscaler. Supplied groups are authoritative (not merged with the default), so at least one of them must carry `roles: [ingress-nginx]` — enforced at render time. Each supplied pool key must be at most 15 characters so its `kubernetes-nodes-computeplane-cluster-<pool>` HelmRelease name fits Helm's 53-character limit — also enforced at render time.
3538
nodeGroups: {}

0 commit comments

Comments
 (0)