mirror of
https://github.com/cozystack/cozystack.git
synced 2026-11-09 14:02:56 +09:00
feat(kubevirt): expose migration configuration through platform values
Backport of #4254 to release-1.6. Cluster-wide migration settings could only be applied by patching the KubeVirt CR: the kubevirt chart did not render `migrations`, and the platform bundle did not forward it. The optional `kubevirt.migrations` platform value now passes through the generated Package to `spec.configuration.migrations`, and a hand patch of that Package is undone on the next platform render, so this hop is the only supported setter. Four differences from main, all from release-1.6 predating the disabledFeatureGates work: - platform values gains a `kubevirt` block holding `migrations` alone, and the iaas bundle gains only the migrations hop; - the wiring test takes the four migrations cases and the case for a null `kubevirt` block, without the assertions that belong to disabledFeatureGates cases absent here; - update_idempotency_test.sh gains `run_update_logged`, which arrived on main alongside that work and the new cases need; - the kubevirt Makefile header counts six sed patches rather than five. Verified on this branch: 9 kubevirt chart tests, 31 platform wiring tests (102 across the platform suite), and update_idempotency_test PASS under GNU make and sed. Removing the new awk guard from the Makefile turns that test red naming the migrations block, so the ported guard is not decorative. On this line (.Values.kubevirt).migrations is the only reader of .Values.kubevirt, and rewriting it as .Values.kubevirt.migrations turns the null-block case red with a nil pointer. The automated backport in #4322 stopped with conflict markers in four files as its only commit, which is why this exists. Assisted-by: LLM Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com>
This commit is contained in:
1 parent
8a6ce8121d
commit
d45eb9bfec
8 files changed
+285
-2
No files matched your search
@@ -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
|
||||
|
||||
@@ -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:
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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; }
|
||||
@@ -32,6 +32,10 @@ spec:
|
||||
mediatedDevicesConfiguration:
|
||||
{{- toYaml . | nindent 6 }}
|
||||
{{- end }}
|
||||
{{- with .Values.migrations }}
|
||||
migrations:
|
||||
{{- toYaml . | nindent 6 }}
|
||||
{{- end }}
|
||||
evictionStrategy: LiveMigrate
|
||||
virtualMachineOptions:
|
||||
disableSerialConsoleLog: {}
|
||||
|
||||
@@ -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:
|
||||
|
||||
@@ -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"
|
||||
@@ -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: {}
|
||||
Reference in new issue
Block a user