Repository navigation
kubernetes-nodes: the partial resources.cpu/memory check only runs when instanceType is set #4546
Description
A kubernetes-nodes pool with instanceType: "" and only one of resources.cpu or resources.memory renders without an error. The docs say the chart rejects it. The field description in values.yaml:66, the README and the openAPISchema in the kubernetes-nodes cozyrd all say "Set both cpu and memory together or neither; setting only one is rejected at render time."
The check at packages/apps/kubernetes-nodes/templates/nodegroup.yaml:618 on main (6d759cc) runs only when instanceType is set:
{{- if and $group.instanceType (or (and $hasCpu (not $hasMem)) (and $hasMem (not $hasCpu))) }}
{{- fail (printf "nodeGroup %s: set both resources.cpu and resources.memory, or neither" $groupName) }}Repro from packages/apps/kubernetes-nodes, with the values the helm-unittest suites use:
helm template kubernetes-nodes-myk8s-md0 . --namespace tenant-test \
--set cluster=myk8s --set _cluster.cluster-domain=cozy.local --set version=v1.35 \
--set minReplicas=0 --set maxReplicas=3 --set instanceType= --set resources.cpu=2 \
--show-only templates/nodegroup.yaml
instanceType="" resources.cpu=2 only -> exit 0, domain.cpu.cores: 2, no memory
instanceType="" resources.memory=4Gi only -> exit 0, domain.memory.guest: 4Gi, no cpu
instanceType=u1.medium resources.cpu=2 only -> exit 1, "set both resources.cpu and resources.memory, or neither"
No test covers the guard. Nothing under packages/apps/kubernetes-nodes/tests asserts that error message.
The comment above the check explains why it exists: with an instancetype, a single domain override is rejected by KubeVirt. That reason does not apply without an instancetype, so the gate may be deliberate and the docs wrong. Or the docs are right and the gate is too narrow. I'd drop the instanceType condition, so a partial spec fails the same way on every pool, and add a helm-unittest case for both halves with instanceType: "". Before that, someone should decide what a pool with neither instanceType nor full resources should do, because today that also renders with no sizing at all. If the gate is intended, the three field descriptions should say it applies only with instanceType.
A second, low-impact problem sits in the same field. cores: {{ .group.resources.cpu }} at nodegroup.yaml:532 prints the value as Helm decoded it. An unquoted YAML integer is decoded as float64, and Go prints a float64 of 1000000 or more in exponent form. With a values file:
resources: {cpu: 999999, memory: 4Gi} -> cores: 999999
resources: {cpu: 1000000, memory: 4Gi} -> cores: 1e+06
resources: {cpu: 1234567, memory: 4Gi} -> cores: 1.234567e+06
resources: {cpu: "1000000", memory: 4Gi} -> cores: 1000000
No real worker has a million vCPUs, so this is cosmetic in practice. I did not check whether the API server accepts 1e+06 in the integer cores field. #4386 adds a range check for resources.cpu but keeps the emitted bytes as they are, so this stays after it too. The fix is to print the decoded integer, for example {{ .group.resources.cpu | int64 }} once the value is known to be a whole number.
Activity
lexfrei commentedon Sep 28, 2026
One more item for the same field: a leading-zero
resources.cpugives the VM one core count and the autoscaler another.With
resources: {cpu: "010", memory: 4Gi}andinstanceType: "", main (6d759cc) renders:cores: 010is copied as written, and YAML 1.1 reads it as octal.sigs.k8s.io/yamlv1.6.0, the version in go.mod, turns it into{"cores":8}. The annotation atnodegroup.yaml:910goes throughkubernetes.cpuToMillicoresintemplates/_resources.tpl, which parses the string as decimal 10, so the 9000m is 10 cores minus the reserve. The autoscaler then plans for 10 cores on a node that has 8. The proxmoxnumCoresatnodegroup.yaml:144also reads 10."08"is not valid octal, so it decodes to 8 on both sides and is consistent.This is a narrow input and #4386 keeps it on purpose, because existing pools may carry it and changing the output would roll them. The fix is to take the effective CPU from the YAML-decoded value (the same decode #4386 already does for its range check) for every place that reads it. The other way is to refuse leading zeros, but that needs a migration for pools that already have one.
themoriarti commentedon Oct 19, 2026
Fix in #4973: the partial check runs on every kubevirt pool, a pool with neither sizing source is refused at render with a message naming both (the decision this issue asked for), and the float64 exponent case prints an integer. Byte stability for the other spellings is checked in the PR. The leading-zero item from your comment is handled together with #4554 in a separate PR.
fix(kubernetes-nodes): plan autoscaler capacity on the cores the VM a…
fix(kubernetes-nodes): refuse a partial resources spec on every pool
fix(kubernetes-nodes): refuse a partial resources spec on every pool (c…
fix(kubernetes-nodes): plan autoscaler capacity on the cores the VM a…