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 <moriarti@cp.if.ua>
Assisted-by: LLM

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. */}}

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

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