From 12110956939db611ffe0e86e0d2db8d55705fddb Mon Sep 17 00:00:00 2001 From: Marian Koreniuk Date: Mon, 19 Oct 2026 23:29:09 +0200 Subject: [PATCH] fix(kubernetes-nodes): refuse a partial resources spec on every pool The resources.cpu/resources.memory pair check ran only when an instanceType was set, while the field description promised a render-time error for a partial spec on any pool. Without an instancetype a cpu-only pool renders a VM with no memory, which KubeVirt refuses when the Machine is created, and a memory-only pool silently sizes the guest at one vCPU. Run the check on every kubevirt pool; the proxmox branch already requires both. A kubevirt pool with neither an instanceType nor both resources now fails at render with a message naming both, the way vm-instance does; no such pool ever passed admission, so nothing that runs is affected. A float64 core count of one million or more printed in exponent form, which the CRD's integer field cannot take. Print it as an integer; every smaller whole number prints the same bytes, so no template name moves. Closes #4546 Signed-off-by: Marian Koreniuk Assisted-by: LLM --- .../kubernetes-nodes/templates/nodegroup.yaml | 27 +++- .../tests/resources_cpu_test.yaml | 86 +++++++++++-- .../tests/resources_partial_test.yaml | 121 ++++++++++++++++++ 3 files changed, 222 insertions(+), 12 deletions(-) create mode 100644 packages/apps/kubernetes-nodes/tests/resources_partial_test.yaml diff --git a/packages/apps/kubernetes-nodes/templates/nodegroup.yaml b/packages/apps/kubernetes-nodes/templates/nodegroup.yaml index 9060f97a3..5c2948b2e 100644 --- a/packages/apps/kubernetes-nodes/templates/nodegroup.yaml +++ b/packages/apps/kubernetes-nodes/templates/nodegroup.yaml @@ -567,7 +567,13 @@ spec: {{- if and .group.resources .group.resources.cpu }} cpu: threads: 1 - cores: {{ .group.resources.cpu }} + {{- /* An unquoted value arrives as float64, which Go prints in + exponent form from 1e6 upward (1000000 -> 1e+06), and the + CRD field is an integer. %.0f prints the same bytes as the + default formatting for every whole number below that, so + no existing template name moves; a string is emitted as + written, as it always was. */}} + cores: {{ if kindIs "float64" .group.resources.cpu }}{{ printf "%.0f" .group.resources.cpu }}{{ else }}{{ .group.resources.cpu }}{{ end }} sockets: 1 {{- end }} devices: @@ -650,12 +656,27 @@ spec: valid, resource-sized group. Explicit resources are all-or-nothing: a partial spec (only one of cpu/memory) would emit one domain override alongside the instancetype, which KubeVirt rejects, so it fails here with - a clear render-time error instead of a late admission rejection. */}} + a clear render-time error instead of a late admission rejection. The + same rule holds without an instancetype, which is what the field + description promises: cpu alone renders a VM with no memory, which + KubeVirt refuses when the Machine is created, and memory alone sizes the + guest at KubeVirt's default of one vCPU with nothing saying so. The + proxmox branch requires both up front, so this is the kubevirt half of + one rule. Zero is falsy here as everywhere in this chart (see the + resources.cpu check above), so an unquoted zero counts as unset. */}} {{- $hasCpu := and $group.resources $group.resources.cpu }} {{- $hasMem := and $group.resources $group.resources.memory }} -{{- if and $group.instanceType (or (and $hasCpu (not $hasMem)) (and $hasMem (not $hasCpu))) }} +{{- if or (and $hasCpu (not $hasMem)) (and $hasMem (not $hasCpu)) }} {{- fail (printf "nodeGroup %s: set both resources.cpu and resources.memory, or neither" $groupName) }} {{- end }} +{{- /* With neither sizing source the VM has no memory at all, and KubeVirt + refuses it at admission, long after helm has reported success. No pool + could ever have run in this state, so refusing it here costs nothing + and names the choice. Proxmox has no instancetype and checks the same + thing at the top of this file. */}} +{{- if and (not $isProxmox) (not $group.instanceType) (not (and $hasCpu $hasMem)) }} +{{- fail (printf "nodeGroup %s: set instanceType, or both resources.cpu and resources.memory: with neither the worker VM has no size, and KubeVirt refuses a VirtualMachine that declares no memory" $groupName) }} +{{- end }} {{- /* Presence, not truth, for both fields below: zero is a quantity the schema accepts, and a guard keyed on truthiness would wave through exactly the value it exists to catch. */}} diff --git a/packages/apps/kubernetes-nodes/tests/resources_cpu_test.yaml b/packages/apps/kubernetes-nodes/tests/resources_cpu_test.yaml index 9c65c3d45..13d82ffd9 100644 --- a/packages/apps/kubernetes-nodes/tests/resources_cpu_test.yaml +++ b/packages/apps/kubernetes-nodes/tests/resources_cpu_test.yaml @@ -158,14 +158,43 @@ tests: - failedTemplate: errorPattern: 'nodeGroup md0: resources.cpu \(1.5\) must be a whole number of vCPU cores between 1 and 4294967295' - # An unquoted zero is falsy, so it has always rendered as if the field were - # unset: no domain.cpu, sizing left to the instancetype. The template name - # below is what the chart rendered for it before this guard existed. - - it: keeps rendering an unquoted zero as an unset field + # An unquoted zero is falsy, so it has always counted as an unset field. + # Beside a memory value it is therefore a partial spec, which the chart + # refuses on every pool, and alone it leaves the instancetype to size the + # guest: no domain.cpu, and the matcher stays on the VM. + - it: reads an unquoted zero as an unset field set: + instanceType: u1.medium resources: cpu: 0 - memory: 4Gi + # The instancetype the lookup resolves, plus the LIST kinds the + # old-template-preservation loop registers with the fake client, as in + # worker_image_source_test.yaml. + kubernetesProvider: + scheme: + "instancetype.kubevirt.io/v1beta1/VirtualMachineClusterInstancetype": + gvr: {group: instancetype.kubevirt.io, version: v1beta1, resource: virtualmachineclusterinstancetypes} + namespaced: false + "cluster.x-k8s.io/v1beta1/MachineSet": + gvr: {group: cluster.x-k8s.io, version: v1beta1, resource: machinesets} + namespaced: true + "infrastructure.cluster.x-k8s.io/v1alpha1/KubevirtMachineTemplate": + gvr: {group: infrastructure.cluster.x-k8s.io, version: v1alpha1, resource: kubevirtmachinetemplates} + namespaced: true + "cdi.kubevirt.io/v1beta1/DataVolume": + gvr: {group: cdi.kubevirt.io, version: v1beta1, resource: datavolumes} + namespaced: true + objects: + - kind: VirtualMachineClusterInstancetype + apiVersion: instancetype.kubevirt.io/v1beta1 + metadata: {name: u1.medium} + - kind: MachineSet + apiVersion: cluster.x-k8s.io/v1beta1 + metadata: {name: ignored-ms, namespace: tenant-test} + spec: {template: {spec: {infrastructureRef: {kind: Other, name: ignored}}}} + - kind: KubevirtMachineTemplate + apiVersion: infrastructure.cluster.x-k8s.io/v1alpha1 + metadata: {name: ignored-kmt, namespace: tenant-test} documentSelector: path: kind value: KubevirtMachineTemplate @@ -173,8 +202,17 @@ tests: - notExists: path: spec.template.spec.virtualMachineTemplate.spec.template.spec.domain.cpu - equal: - path: metadata.name - value: kubernetes-myk8s-md0-28d812 + path: spec.template.spec.virtualMachineTemplate.spec.instancetype.name + value: u1.medium + + - it: refuses an unquoted zero beside memory on a pool with no instancetype + set: + resources: + cpu: 0 + memory: 4Gi + asserts: + - failedTemplate: + errorPattern: 'nodeGroup md0: set both resources.cpu and resources.memory, or neither' # A quoted zero is truthy, so it never took the unset path. It was refused # before this guard existed as well, by the kubelet CPU reservation check, @@ -234,10 +272,40 @@ tests: - failedTemplate: errorPattern: 'nodeGroup md0: resources.cpu \(1e10\) must be a whole number of vCPU cores between 1 and 4294967295' + # Memory alone is a partial spec on every pool now (resources_partial_test.yaml), + # so the unset field is read on a pool its instancetype sizes. - it: leaves domain.cpu out when resources.cpu is unset set: - resources: - memory: 4Gi + instanceType: u1.medium + resources: {} + # The instancetype the lookup resolves, plus the LIST kinds the + # old-template-preservation loop registers with the fake client, as in + # worker_image_source_test.yaml. + kubernetesProvider: + scheme: + "instancetype.kubevirt.io/v1beta1/VirtualMachineClusterInstancetype": + gvr: {group: instancetype.kubevirt.io, version: v1beta1, resource: virtualmachineclusterinstancetypes} + namespaced: false + "cluster.x-k8s.io/v1beta1/MachineSet": + gvr: {group: cluster.x-k8s.io, version: v1beta1, resource: machinesets} + namespaced: true + "infrastructure.cluster.x-k8s.io/v1alpha1/KubevirtMachineTemplate": + gvr: {group: infrastructure.cluster.x-k8s.io, version: v1alpha1, resource: kubevirtmachinetemplates} + namespaced: true + "cdi.kubevirt.io/v1beta1/DataVolume": + gvr: {group: cdi.kubevirt.io, version: v1beta1, resource: datavolumes} + namespaced: true + objects: + - kind: VirtualMachineClusterInstancetype + apiVersion: instancetype.kubevirt.io/v1beta1 + metadata: {name: u1.medium} + - kind: MachineSet + apiVersion: cluster.x-k8s.io/v1beta1 + metadata: {name: ignored-ms, namespace: tenant-test} + spec: {template: {spec: {infrastructureRef: {kind: Other, name: ignored}}}} + - kind: KubevirtMachineTemplate + apiVersion: infrastructure.cluster.x-k8s.io/v1alpha1 + metadata: {name: ignored-kmt, namespace: tenant-test} documentSelector: path: kind value: KubevirtMachineTemplate diff --git a/packages/apps/kubernetes-nodes/tests/resources_partial_test.yaml b/packages/apps/kubernetes-nodes/tests/resources_partial_test.yaml new file mode 100644 index 000000000..7b2924636 --- /dev/null +++ b/packages/apps/kubernetes-nodes/tests/resources_partial_test.yaml @@ -0,0 +1,121 @@ +suite: worker pool resources are all-or-nothing + +# resources.cpu and resources.memory size the worker VM together or not at +# all. With an instancetype a lone override is refused by KubeVirt; without one +# cpu alone renders a VM with no memory, which KubeVirt refuses at admission, +# and memory alone sizes the guest at one vCPU with nothing saying so. The +# field description promises a render-time error for a partial spec, and this +# file holds the chart to it on every pool. A pool with neither sizing source +# is refused too, the way vm-instance refuses it: no such pool ever passed +# admission, so nothing that runs today is affected. + +templates: + - templates/nodegroup.yaml +release: + name: kubernetes-nodes-myk8s-md0 + namespace: tenant-test +set: + cluster: myk8s + _cluster: + cluster-domain: cozy.local + version: "v1.35" + minReplicas: 0 + maxReplicas: 3 + instanceType: "" + diskSize: 20Gi + storageClass: replicated +tests: + - it: refuses cpu without memory on a pool with no instancetype + set: + resources: + cpu: "2" + asserts: + - failedTemplate: + errorPattern: 'nodeGroup md0: set both resources.cpu and resources.memory, or neither' + + - it: refuses memory without cpu on a pool with no instancetype + set: + resources: + memory: 4Gi + asserts: + - failedTemplate: + errorPattern: 'nodeGroup md0: set both resources.cpu and resources.memory, or neither' + + # The case the check always caught. Pinned so the two halves cannot drift: + # the partial check runs before the instancetype lookup, so no stub is needed. + - it: refuses cpu without memory on a pool sized by an instancetype + set: + instanceType: u1.medium + resources: + cpu: "2" + asserts: + - failedTemplate: + errorPattern: 'nodeGroup md0: set both resources.cpu and resources.memory, or neither' + + - it: refuses memory without cpu on a pool sized by an instancetype + set: + instanceType: u1.medium + resources: + memory: 4Gi + asserts: + - failedTemplate: + errorPattern: 'nodeGroup md0: set both resources.cpu and resources.memory, or neither' + + - it: refuses a pool with neither an instancetype nor explicit resources + asserts: + - failedTemplate: + errorPattern: 'nodeGroup md0: set instanceType, or both resources.cpu and resources.memory' + + - it: refuses an empty resources block beside an empty instancetype + set: + resources: {} + asserts: + - failedTemplate: + errorPattern: 'nodeGroup md0: set instanceType, or both resources.cpu and resources.memory' + + - it: sizes the guest from both fields when both are set + set: + resources: + cpu: "2" + memory: 4Gi + documentSelector: + path: kind + value: KubevirtMachineTemplate + asserts: + - equal: + path: spec.template.spec.virtualMachineTemplate.spec.template.spec.domain.cpu.cores + value: 2 + - equal: + path: spec.template.spec.virtualMachineTemplate.spec.template.spec.domain.memory.guest + value: 4Gi + - notExists: + path: spec.template.spec.virtualMachineTemplate.spec.template.spec.instancetype + + # An unquoted value is decoded as float64, which Go prints in exponent form + # from one million upward. The CRD field is an integer, so the chart prints + # the whole number; every smaller value prints the same bytes as before. + - it: prints a float64 core count of a million as an integer + set: + resources: + cpu: 1000000 + memory: 4Gi + documentSelector: + path: kind + value: KubevirtMachineTemplate + asserts: + - equal: + path: spec.template.spec.virtualMachineTemplate.spec.template.spec.domain.cpu.cores + value: 1000000 + + - it: prints a whole float64 below a million the way it always did + set: + resources: + cpu: 4.0 + memory: 4Gi + documentSelector: + path: kind + value: KubevirtMachineTemplate + asserts: + - equal: + path: spec.template.spec.virtualMachineTemplate.spec.template.spec.domain.cpu.cores + value: 4