diff --git a/packages/core/platform/templates/bundles/iaas.yaml b/packages/core/platform/templates/bundles/iaas.yaml index 25215c321..1f8f11b8b 100644 --- a/packages/core/platform/templates/bundles/iaas.yaml +++ b/packages/core/platform/templates/bundles/iaas.yaml @@ -6,6 +6,17 @@ {{- if .Values.resources.cpuAllocationRatio -}} {{- $_ := set $kubevirtValues "cpuAllocationRatio" (.Values.resources.cpuAllocationRatio | int) -}} {{- end -}} +{{- /* + KubeVirt CR settings that no other platform knob implies, forwarded + verbatim. This include is the only writer of the cozystack.kubevirt Package, + and patching that Package by hand is reverted the next time the platform + re-renders it, so without this hop the chart value has no supported setter. + The parenthesized walk is nil-safe for the reason the gpu block below spells + out. +*/ -}} +{{- with (.Values.kubevirt).migrations -}} +{{- $_ := set $kubevirtValues "migrations" . -}} +{{- end -}} {{- /* When cozystack.gpu-operator is enabled the platform mirrors the variant into the KubeVirt CR: the HostDevices feature gate plus a starter diff --git a/packages/core/platform/tests/bundles_gpu_kubevirt_wiring_test.yaml b/packages/core/platform/tests/bundles_gpu_kubevirt_wiring_test.yaml index 9f6077ff9..7e06f6bf2 100644 --- a/packages/core/platform/tests/bundles_gpu_kubevirt_wiring_test.yaml +++ b/packages/core/platform/tests/bundles_gpu_kubevirt_wiring_test.yaml @@ -386,6 +386,118 @@ tests: path: spec.components.kubevirt.values.cpuAllocationRatio value: 7 + - it: forwards kubevirt.migrations into the kubevirt component values + set: + bundles: + system: + enabled: true + variant: isp-full + iaas: + enabled: true + enabledPackages: [] + kubevirt: + migrations: + bandwidthPerMigration: 625M + parallelMigrationsPerCluster: 2 + parallelOutboundMigrationsPerNode: 1 + documentSelector: + path: metadata.name + value: cozystack.kubevirt + asserts: + - equal: + path: spec.components.kubevirt.values + value: + cpuAllocationRatio: 10 + migrations: + bandwidthPerMigration: 625M + parallelMigrationsPerCluster: 2 + parallelOutboundMigrationsPerNode: 1 + + - it: omits empty kubevirt.migrations + set: + bundles: + system: + enabled: true + variant: isp-full + iaas: + enabled: true + enabledPackages: [] + kubevirt: + migrations: {} + documentSelector: + path: metadata.name + value: cozystack.kubevirt + asserts: + - equal: + path: spec.components.kubevirt.values + value: + cpuAllocationRatio: 10 + + - it: omits null kubevirt.migrations + set: + bundles: + system: + enabled: true + variant: isp-full + iaas: + enabled: true + enabledPackages: [] + kubevirt: + migrations: null + documentSelector: + path: metadata.name + value: cozystack.kubevirt + asserts: + - equal: + path: spec.components.kubevirt.values + value: + cpuAllocationRatio: 10 + + - it: renders the kubevirt Package without a nil-pointer error when the whole kubevirt block is set to null + set: + bundles: + system: + enabled: true + variant: isp-full + iaas: + enabled: true + enabledPackages: [] + kubevirt: null + documentSelector: + path: metadata.name + value: cozystack.kubevirt + asserts: + - equal: + path: kind + value: Package + - notExists: + path: spec.components.kubevirt.values.migrations + + - it: preserves false and zero kubevirt.migrations values + set: + bundles: + system: + enabled: true + variant: isp-full + iaas: + enabled: true + enabledPackages: [] + kubevirt: + migrations: + allowAutoConverge: false + completionTimeoutPerGiB: 0 + documentSelector: + path: metadata.name + value: cozystack.kubevirt + asserts: + - equal: + path: spec.components.kubevirt.values + value: + cpuAllocationRatio: 10 + migrations: + allowAutoConverge: false + completionTimeoutPerGiB: 0 + - it: appends user-supplied pciHostDevices in vgpu mode so post-kubevirt#16890 SR-IOV vGPU VFs can be advertised alongside the platform mediated defaults set: bundles: diff --git a/packages/core/platform/values.yaml b/packages/core/platform/values.yaml index d5d27e391..d10ca3633 100644 --- a/packages/core/platform/values.yaml +++ b/packages/core/platform/values.yaml @@ -511,6 +511,14 @@ resources: cpuAllocationRatio: 10 memoryAllocationRatio: 1 ephemeralStorageAllocationRatio: 40 +# KubeVirt CR settings that no other platform knob implies. Forwarded into the +# emitted cozystack.kubevirt Package CR as components.kubevirt.values, next to +# the entries the iaas bundle derives from resources.cpuAllocationRatio and +# from the gpu block below. +kubevirt: + # Passed to spec.configuration.migrations on the KubeVirt CR. + # Empty keeps KubeVirt's defaults. bandwidthPerMigration is bytes/second. + migrations: {} # GPU host-device wiring for VirtualMachine workloads. Auto-injected into the # KubeVirt CR's permittedHostDevices when bundles.enabledPackages contains # cozystack.gpu-operator. Without it the admission webhook rejects any VM diff --git a/packages/system/kubevirt/Makefile b/packages/system/kubevirt/Makefile index 516da1838..efb2cf541 100644 --- a/packages/system/kubevirt/Makefile +++ b/packages/system/kubevirt/Makefile @@ -27,7 +27,7 @@ CR_FILE ?= templates/kubevirt-cr.yaml # 1. Operator inspects the new upstream release notes for spec changes # that affect the cozystack-managed fields listed above. # 2. Operator hand-merges the new fields into templates/kubevirt-cr.yaml. -# 3. Operator runs `make update` to re-apply the five cozystack +# 3. Operator runs `make update` to re-apply the six cozystack # parameterization sed patches in case they were lost during the # hand-merge. # @@ -46,6 +46,7 @@ update: @# sanity-check tail below. grep -q '{{- with .Values.permittedHostDevices }}' $(CR_FILE) || sed -i '/^ evictionStrategy:/i\ {{- with .Values.permittedHostDevices }}\n permittedHostDevices:\n {{- toYaml . | nindent 6 }}\n {{- end }}' $(CR_FILE) grep -q '{{- with .Values.mediatedDevicesConfiguration }}' $(CR_FILE) || sed -i '/^ evictionStrategy:/i\ {{- with .Values.mediatedDevicesConfiguration }}\n mediatedDevicesConfiguration:\n {{- toYaml . | nindent 6 }}\n {{- end }}' $(CR_FILE) + grep -q '{{- with .Values.migrations }}' $(CR_FILE) || sed -i '/^ evictionStrategy:/i\ {{- with .Values.migrations }}\n migrations:\n {{- toYaml . | nindent 6 }}\n {{- end }}' $(CR_FILE) @# Sanity-check: every cozystack patch MUST end up in the file. If the @# hand-merge step moved or renamed an anchor (developerConfiguration:, @# - VMExport, evictionStrategy:) the sed silently no-ops and the chart @@ -56,3 +57,11 @@ update: @grep -q '{{- range .Values.extraFeatureGates }}' $(CR_FILE) || { echo "ERROR: directive '{{- range .Values.extraFeatureGates }}' not inserted — restore the '- VMExport' anchor in $(CR_FILE) and re-run" >&2; exit 1; } @grep -q '{{- with .Values.permittedHostDevices }}' $(CR_FILE) || { echo "ERROR: directive '{{- with .Values.permittedHostDevices }}' not inserted — restore the 'evictionStrategy:' anchor in $(CR_FILE) and re-run" >&2; exit 1; } @grep -q '{{- with .Values.mediatedDevicesConfiguration }}' $(CR_FILE) || { echo "ERROR: directive '{{- with .Values.mediatedDevicesConfiguration }}' not inserted — restore the 'evictionStrategy:' anchor in $(CR_FILE) and re-run" >&2; exit 1; } + @grep -q '{{- with .Values.migrations }}' $(CR_FILE) || { echo "ERROR: directive '{{- with .Values.migrations }}' not inserted — restore the 'evictionStrategy:' anchor in $(CR_FILE) and re-run" >&2; exit 1; } + @# A retained directive skips insertion even if its body or end was lost. + @# Check the whole block in order; only END decides the exit status. + @awk '\ + $$0 == " migrations:" { keys++ } \ + $$0 == " {{- with .Values.migrations }}" { blocks++; step=1; next } \ + step { expected = step == 1 ? " migrations:" : step == 2 ? " {{- toYaml . | nindent 6 }}" : " {{- end }}"; if ($$0 != expected) bad=1; step=(step+1)%4 } \ + END { exit !(blocks == 1 && keys == 1 && step == 0 && !bad) }' $(CR_FILE) || { echo "ERROR: migrations block is partial or duplicated — restore the complete block or delete all its remaining lines from $(CR_FILE) and re-run" >&2; exit 1; } diff --git a/packages/system/kubevirt/templates/kubevirt-cr.yaml b/packages/system/kubevirt/templates/kubevirt-cr.yaml index 10754aad3..6b7c0b06d 100644 --- a/packages/system/kubevirt/templates/kubevirt-cr.yaml +++ b/packages/system/kubevirt/templates/kubevirt-cr.yaml @@ -32,6 +32,10 @@ spec: mediatedDevicesConfiguration: {{- toYaml . | nindent 6 }} {{- end }} + {{- with .Values.migrations }} + migrations: + {{- toYaml . | nindent 6 }} + {{- end }} evictionStrategy: LiveMigrate virtualMachineOptions: disableSerialConsoleLog: {} diff --git a/packages/system/kubevirt/tests/kubevirt_cr_test.yaml b/packages/system/kubevirt/tests/kubevirt_cr_test.yaml index eda3218ca..556524e67 100644 --- a/packages/system/kubevirt/tests/kubevirt_cr_test.yaml +++ b/packages/system/kubevirt/tests/kubevirt_cr_test.yaml @@ -23,6 +23,48 @@ tests: path: spec.configuration.permittedHostDevices - notExists: path: spec.configuration.mediatedDevicesConfiguration + - notExists: + path: spec.configuration.migrations + + - it: omits migrations when explicitly empty + set: + migrations: {} + asserts: + - notExists: + path: spec.configuration.migrations + + - it: omits migrations when explicitly null + set: + migrations: null + asserts: + - notExists: + path: spec.configuration.migrations + + - it: renders migration limits without changing their values + set: + migrations: + bandwidthPerMigration: 625M + parallelMigrationsPerCluster: 2 + parallelOutboundMigrationsPerNode: 1 + asserts: + - equal: + path: spec.configuration.migrations + value: + bandwidthPerMigration: 625M + parallelMigrationsPerCluster: 2 + parallelOutboundMigrationsPerNode: 1 + + - it: preserves false and zero migration values + set: + migrations: + allowAutoConverge: false + completionTimeoutPerGiB: 0 + asserts: + - equal: + path: spec.configuration.migrations + value: + allowAutoConverge: false + completionTimeoutPerGiB: 0 - it: emits cpuAllocationRatio under developerConfiguration when set set: diff --git a/packages/system/kubevirt/tests/update_idempotency_test.sh b/packages/system/kubevirt/tests/update_idempotency_test.sh index 8bf373d40..a698d8016 100755 --- a/packages/system/kubevirt/tests/update_idempotency_test.sh +++ b/packages/system/kubevirt/tests/update_idempotency_test.sh @@ -50,11 +50,17 @@ run_update() { make -C "$pkg" update CR_FILE="$1" >/dev/null 2>&1 } +# Same call, but keeps stdout+stderr so a case can assert on the diagnostic. +run_update_logged() { + make -C "$pkg" update CR_FILE="$1" >"$2" 2>&1 +} + tmpdir="$(mktemp -d)" trap 'rm -rf "$tmpdir"' EXIT PHD='{{- with .Values.permittedHostDevices }}' MDC='{{- with .Values.mediatedDevicesConfiguration }}' +MIG='{{- with .Values.migrations }}' # --- case 1: a hand-merge dropped ONLY the mediatedDevicesConfiguration block. # `make update` must reinsert it without duplicating permittedHostDevices. @@ -96,8 +102,92 @@ for guard in \ '{{- if .Values.cpuAllocationRatio }}' \ '{{- range .Values.extraFeatureGates }}' \ "$PHD" \ - "$MDC"; do + "$MDC" \ + "$MIG"; do assert_count 1 "$guard" "$missing_mdev" done +# A hand-merge may drop migrations while preserving both device blocks. +# Restoring it must reproduce the committed template, including the YAML key +# and indentation, rather than merely reintroduce the directive. +missing_migrations="$tmpdir/missing-migrations.yaml" +sed '/{{- with .Values.migrations }}/,/{{- end }}/d' "$src" >"$missing_migrations" +assert_absent "$MIG" "$missing_migrations" +assert_present "$PHD" "$missing_migrations" +assert_present "$MDC" "$missing_migrations" +if ! run_update "$missing_migrations"; then + fail "make update exited non-zero on a template missing only migrations" +fi +assert_present "$MIG" "$missing_migrations" +if ! diff -u "$src" "$missing_migrations" >/dev/null; then + fail "make update rebuilt migrations differently from the committed template" +fi +run_update "$missing_migrations" +assert_count 1 "$MIG" "$missing_migrations" +if ! diff -u "$src" "$missing_migrations" >/dev/null; then + fail "repeated make update changed the restored migrations template" +fi + +# sed succeeds even when its insertion anchor is absent; the target's sanity +# check must reject a hand-merged CR that cannot restore migration settings. +anchorless_migrations="$tmpdir/anchorless-migrations.yaml" +sed -e '/{{- with .Values.migrations }}/,/{{- end }}/d' \ + -e '/^ evictionStrategy:/d' "$src" >"$anchorless_migrations" +anchorless_migrations_log="$tmpdir/anchorless-migrations.log" +if run_update_logged "$anchorless_migrations" "$anchorless_migrations_log"; then + fail "make update exited zero without the migrations block or its evictionStrategy anchor" +fi +if ! grep -qF "directive '$MIG' not inserted" "$anchorless_migrations_log"; then + fail "make update did not name the missing migrations directive: $(cat "$anchorless_migrations_log")" +fi + +# A retained opening directive must not hide a missing body or terminator. +# The opening-less case also leaves an orphan key: inserting a second complete +# block must not make that damaged template pass validation. +for missing in key payload end body opening; do + partial_migrations="$tmpdir/migrations-missing-$missing.yaml" + awk -v missing="$missing" -v opening="$MIG" ' + index($0, opening) { inside=1; if (missing != "opening") print; next } + inside && /{{- end }}/ { inside=0; if (missing != "end") print; next } + inside && /^ migrations:$/ && (missing == "key" || missing == "body") { next } + inside && /{{- toYaml \. \| nindent 6 }}/ && (missing == "payload" || missing == "body") { next } + { print } + ' "$src" >"$partial_migrations" + partial_migrations_log="$tmpdir/migrations-missing-$missing.log" + if run_update_logged "$partial_migrations" "$partial_migrations_log"; then + fail "make update exited zero on a migrations block missing $missing" + fi + if ! grep -qF 'migrations block is partial or duplicated' "$partial_migrations_log"; then + fail "make update did not diagnose migrations missing $missing: $(cat "$partial_migrations_log")" + fi +done + +# Presence checks can also accept reordered lines, a duplicate complete block, +# or a complete body with EOF in place of its closing directive. +for malformed in reordered duplicate eof; do + malformed_migrations="$tmpdir/migrations-$malformed.yaml" + awk -v malformed="$malformed" -v opening="$MIG" ' + index($0, opening) { inside=1; block=$0 ORS; print; next } + inside { + block=block $0 ORS + if (/{{- end }}/) { + if (malformed == "eof") exit + inside=0; print + if (malformed == "duplicate") printf "%s", block + next + } + if (malformed == "reordered" && /^ migrations:$/) { print " {{- toYaml . | nindent 6 }}"; next } + if (malformed == "reordered" && /{{- toYaml \. \| nindent 6 }}/) { print " migrations:"; next } + } + { print } + ' "$src" >"$malformed_migrations" + malformed_migrations_log="$tmpdir/migrations-$malformed.log" + if run_update_logged "$malformed_migrations" "$malformed_migrations_log"; then + fail "make update exited zero on a $malformed migrations block" + fi + if ! grep -qF 'migrations block is partial or duplicated' "$malformed_migrations_log"; then + fail "make update did not diagnose $malformed migrations: $(cat "$malformed_migrations_log")" + fi +done + echo "PASS update_idempotency_test" diff --git a/packages/system/kubevirt/values.yaml b/packages/system/kubevirt/values.yaml index 5a8bb493b..57d1beefc 100644 --- a/packages/system/kubevirt/values.yaml +++ b/packages/system/kubevirt/values.yaml @@ -46,3 +46,10 @@ permittedHostDevices: {} # mediatedDeviceTypes: # - nvidia-740 mediatedDevicesConfiguration: {} + +# Passed to spec.configuration.migrations on the KubeVirt CR. +# Empty leaves KubeVirt's migration defaults unchanged. +# bandwidthPerMigration is a quantity in bytes per second. +# Set through spec.components.platform.values.kubevirt.migrations +# on the cozystack.cozystack-platform Package. +migrations: {}