Skip to content

Commit 4e531b1

Browse files
lexfreilexfrei
committed
fix(kubernetes): make node group resources optional and prefer them over instanceType
A node group is sized either by instanceType or by explicit resources, but the generated values schema required resources on every node group. A node group with instanceType and no resources — or an empty resources dropped during typed-CR to HelmRelease translation — failed schema validation with "missing property 'resources'" even though sizing was fully specified. Mark resources optional in the node group definition and regenerate the schema, API types, README, and ApplicationDefinition. The chart template emits domain.cpu/memory only when resources is present, so a node group sized purely by instanceType now passes validation and installs. When a node group sets both instanceType and explicit resources, explicit resources take precedence: the instancetype is omitted from the rendered VM and the per-group instancetype lookup is skipped, so KubeVirt accepts the explicit domain.cpu/memory with no instancetype to conflict with. instanceType stays required and defaulted (no schema change), so the console's required instanceType dropdown keeps working — sending the default instanceType plus explicit resources simply sizes the VM by resources. This replaces an earlier render-time hard failure on the both-set case, which was unreachable to clear from the dashboard (the required dropdown cannot be emptied). Explicit resources are all-or-nothing alongside an instanceType: a partial spec (only cpu, or only memory) on a group that carries a non-empty instanceType would emit one domain override next to the instancetype — which KubeVirt rejects — so it now fails at render time with a clear error. Partial resources on a group without an instanceType (used to tune kubelet reservations) are unaffected. Assisted-By: Claude <[email protected]> Signed-off-by: Aleksei Sviridkin <[email protected]>
1 parent 4404c5c commit 4e531b1

7 files changed

Lines changed: 191 additions & 33 deletions

File tree

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

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

‎packages/apps/kubernetes/README.md‎

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

‎packages/apps/kubernetes/templates/cluster.yaml‎

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -70,10 +70,16 @@ spec:
7070
node-role.kubernetes.io/{{ . }}: ""
7171
{{- end }}
7272
spec:
73-
{{- with .group.instanceType }}
73+
{{- /* Explicit resources take precedence over instanceType. When a node
74+
group is sized by resources (both cpu and memory set), the
75+
instancetype is omitted from the VM: KubeVirt rejects a
76+
VirtualMachine that references an instancetype and also overrides
77+
either domain.cpu or domain.memory. */}}
78+
{{- $sizedByResources := and .group.resources .group.resources.cpu .group.resources.memory }}
79+
{{- if and .group.instanceType (not $sizedByResources) }}
7480
instancetype:
7581
kind: VirtualMachineClusterInstancetype
76-
name: {{ . }}
82+
name: {{ .group.instanceType }}
7783
{{- end }}
7884
runStrategy: Always
7985
dataVolumeTemplates:
@@ -406,9 +412,24 @@ metadata:
406412
name: {{ .Release.Name }}
407413
namespace: {{ .Release.Namespace }}
408414
{{- range $groupName, $group := .Values.nodeGroups }}
415+
{{- /* instanceType and explicit resources are alternative sizing sources, and
416+
explicit resources win. KubeVirt rejects a VirtualMachine that references
417+
an instancetype while also overriding either domain.cpu or domain.memory,
418+
so when a group is sized by resources (both cpu and memory set) the
419+
instancetype is omitted from the VM (see the VM spec above) and the lookup
420+
below is skipped — a stale instancetype name cannot fail an otherwise
421+
valid, resource-sized group. Explicit resources are all-or-nothing: a
422+
partial spec (only one of cpu/memory) would emit one domain override
423+
alongside the instancetype, which KubeVirt rejects, so it fails here with
424+
a clear render-time error instead of a late admission rejection. */}}
425+
{{- $hasCpu := and $group.resources $group.resources.cpu }}
426+
{{- $hasMem := and $group.resources $group.resources.memory }}
427+
{{- if and $group.instanceType (or (and $hasCpu (not $hasMem)) (and $hasMem (not $hasCpu))) }}
428+
{{- fail (printf "nodeGroup %s: set both resources.cpu and resources.memory, or neither" $groupName) }}
429+
{{- end }}
409430
{{/* Resolve instanceType once per group — reused for kubelet reservations and capacity annotations. */}}
410431
{{- $instanceType := dict }}
411-
{{- if $group.instanceType }}
432+
{{- if and $group.instanceType (not (and $hasCpu $hasMem)) }}
412433
{{- $instanceType = (lookup "instancetype.kubevirt.io/v1beta1" "VirtualMachineClusterInstancetype" "" $group.instanceType) }}
413434
{{- if not $instanceType }}
414435
{{- fail (printf "nodeGroup %s: specified instanceType %q not found in cluster" $groupName $group.instanceType) }}

‎packages/apps/kubernetes/tests/cluster_test.yaml‎

Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -292,3 +292,141 @@ tests:
292292
asserts:
293293
- failedTemplate:
294294
errorMessage: 'nodeGroup "md0": ephemeralStorage is no longer supported and should have been automatically migrated to diskSize by platform migration 41. If you see this error after upgrading, the migration did not run — check the cozystack-migration-hook Job logs in cozy-system.'
295+
296+
###############################################
297+
# resources is optional when instanceType set #
298+
###############################################
299+
300+
# A node group may be sized either by instanceType or by explicit resources.
301+
# When instanceType is the sizing source, resources may be omitted entirely.
302+
# helm-unittest enforces values.schema.json, so the render below only succeeds
303+
# if the schema does NOT require resources on each node group.
304+
#
305+
# instanceType is set to "" here only to bypass the live-cluster lookup() that
306+
# validates a real VirtualMachineClusterInstancetype; the schema acceptance of
307+
# an omitted resources key is independent of the instanceType value.
308+
#
309+
# The "worker" group sorts after "md0", so its KubevirtMachineTemplate is
310+
# document index 8 (md0 → 4, worker → 8), matching the multi-group layout above.
311+
312+
- it: accepts a node group that omits resources and renders no explicit cpu/memory
313+
release:
314+
name: test-k8s
315+
namespace: tenant-test
316+
set:
317+
nodeGroups:
318+
worker:
319+
minReplicas: 1
320+
maxReplicas: 3
321+
instanceType: ""
322+
diskSize: "20Gi"
323+
roles:
324+
- worker
325+
asserts:
326+
- isKind:
327+
of: KubevirtMachineTemplate
328+
documentIndex: 8
329+
- equal:
330+
path: spec.template.spec.virtualMachineTemplate.spec.template.metadata.labels["cluster.x-k8s.io/deployment-name"]
331+
value: test-k8s-worker
332+
documentIndex: 8
333+
- notExists:
334+
path: spec.template.spec.virtualMachineTemplate.spec.template.spec.domain.cpu
335+
documentIndex: 8
336+
- notExists:
337+
path: spec.template.spec.virtualMachineTemplate.spec.template.spec.domain.memory
338+
documentIndex: 8
339+
340+
# Complementary case: when explicit resources ARE provided (md0 in common.yaml
341+
# has cpu: 2 / memory: 4Gi), they take effect on the VM domain. Confirms the fix
342+
# only relaxes the schema requirement and does not drop honoured resources.
343+
- it: still renders explicit cpu and memory when resources are provided
344+
release:
345+
name: test-k8s
346+
namespace: tenant-test
347+
asserts:
348+
- isKind:
349+
of: KubevirtMachineTemplate
350+
documentIndex: 4
351+
- equal:
352+
path: spec.template.spec.virtualMachineTemplate.spec.template.spec.domain.cpu.cores
353+
value: 2
354+
documentIndex: 4
355+
- equal:
356+
path: spec.template.spec.virtualMachineTemplate.spec.template.spec.domain.memory.guest
357+
value: 4Gi
358+
documentIndex: 4
359+
360+
# instanceType and explicit resources are alternative sizing sources, and
361+
# explicit resources take precedence. When both are set the VM is sized by the
362+
# explicit resources and the instancetype is omitted, so KubeVirt does not
363+
# reject a VM that both references an instancetype and overrides domain.cpu/memory.
364+
# (md0 in common.yaml already carries cpu: 2 / memory: 4Gi; this only adds an
365+
# instanceType on top to exercise the both-set case.)
366+
- it: prefers explicit resources over instanceType when both are set
367+
release:
368+
name: test-k8s
369+
namespace: tenant-test
370+
set:
371+
nodeGroups.md0.instanceType: "u1.medium"
372+
asserts:
373+
- isKind:
374+
of: KubevirtMachineTemplate
375+
documentIndex: 4
376+
- equal:
377+
path: spec.template.spec.virtualMachineTemplate.spec.template.spec.domain.cpu.cores
378+
value: 2
379+
documentIndex: 4
380+
- equal:
381+
path: spec.template.spec.virtualMachineTemplate.spec.template.spec.domain.memory.guest
382+
value: 4Gi
383+
documentIndex: 4
384+
- notExists:
385+
path: spec.template.spec.virtualMachineTemplate.spec.instancetype
386+
documentIndex: 4
387+
388+
# Explicit resources are all-or-nothing when an instanceType is present. A
389+
# partial spec (only cpu, or only memory) on a group that also carries a
390+
# non-empty instanceType would emit one domain override alongside the
391+
# instancetype, which KubeVirt rejects. Fail fast with a clear render-time
392+
# error instead of shipping a VM the admission webhook rejects at apply time.
393+
# (Fresh group names are used so the partial spec is real — a `set` on md0
394+
# would deep-merge with common.yaml's md0, which already carries both cpu and
395+
# memory.)
396+
- it: fails when only resources.cpu is set without resources.memory
397+
release:
398+
name: test-k8s
399+
namespace: tenant-test
400+
set:
401+
nodeGroups:
402+
mdcpuonly:
403+
minReplicas: 0
404+
maxReplicas: 10
405+
instanceType: "u1.medium"
406+
diskSize: 20Gi
407+
roles:
408+
- worker
409+
resources:
410+
cpu: 2
411+
asserts:
412+
- failedTemplate:
413+
errorMessage: "nodeGroup mdcpuonly: set both resources.cpu and resources.memory, or neither"
414+
415+
- it: fails when only resources.memory is set without resources.cpu
416+
release:
417+
name: test-k8s
418+
namespace: tenant-test
419+
set:
420+
nodeGroups:
421+
mdmemonly:
422+
minReplicas: 0
423+
maxReplicas: 10
424+
instanceType: "u1.medium"
425+
diskSize: 20Gi
426+
roles:
427+
- worker
428+
resources:
429+
memory: 4Gi
430+
asserts:
431+
- failedTemplate:
432+
errorMessage: "nodeGroup mdmemonly: set both resources.cpu and resources.memory, or neither"

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

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,7 @@
4040
"diskSize",
4141
"instanceType",
4242
"maxReplicas",
43-
"minReplicas",
44-
"resources"
43+
"minReplicas"
4544
],
4645
"properties": {
4746
"diskSize": {
@@ -128,7 +127,7 @@
128127
"default": 0
129128
},
130129
"resources": {
131-
"description": "CPU and memory resources for each worker node.",
130+
"description": "Explicit CPU and memory for each worker node, as an alternative to `instanceType` sizing. Optional: when omitted, the node is sized by `instanceType`. When both `cpu` and `memory` are set, they take precedence and `instanceType` is ignored for that node group (the instancetype is omitted from the VM, since KubeVirt cannot override an instancetype's CPU/memory). Set both `cpu` and `memory` together or neither; setting only one is rejected at render time.",
132131
"type": "object",
133132
"properties": {
134133
"cpu": {

‎packages/apps/kubernetes/values.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ storageClass: replicated
8585
## @field {string} [storageClass] - StorageClass for worker node persistent disks. When empty, uses the management cluster default StorageClass (the one annotated storageclass.kubernetes.io/is-default-class: true). NOTE: deliberately not marked immutable — the field is optional and undefaulted, so a strict `self == oldSelf` rule would block any future attempt to set it on an existing node group.
8686
## @x-cozystack-options {source: storageclass}
8787
## @field {[]string} roles - List of node roles.
88-
## @field {Resources} resources - CPU and memory resources for each worker node.
88+
## @field {Resources} [resources] - Explicit CPU and memory for each worker node, as an alternative to `instanceType` sizing. Optional: when omitted, the node is sized by `instanceType`. When both `cpu` and `memory` are set, they take precedence and `instanceType` is ignored for that node group (the instancetype is omitted from the VM, since KubeVirt cannot override an instancetype's CPU/memory). Set both `cpu` and `memory` together or neither; setting only one is rejected at render time.
8989
## @field {[]GPU} gpus - List of GPUs to attach (NVIDIA driver requires at least 4 GiB RAM).
9090
## @field {Kubelet} [kubelet] - Kubelet resource reservations for this node group.
9191

‎packages/system/kubernetes-rd/cozyrds/kubernetes.yaml‎

Lines changed: 1 addition & 1 deletion
Large diffs are not rendered by default.

0 commit comments

Comments
 (0)