Immediate binding for every source fixed a real bug: a standalone disk
on a WaitForFirstConsumer class has no consumer to wait for, so it never
populated. It also took away the path that is correct for the other case.
For a disk a VMInstance consumes, KubeVirt renders a temporary launcher
pod carrying the VM's own affinity and nodeSelector, so the volume binds
where the VM can actually run; with the annotation always present the CDI
worker becomes the first consumer instead and the volume lands where that
pod happened to be scheduled. On a node-pinned class the VM is then
unschedulable against a node-affinity conflict, and storageClass is
immutable, so the only exit is deleting the disk.
The two cases want opposite defaults and only the operator knows which
one a given disk is. bindImmediately defaults to true, so nothing moves
for existing disks or for anyone on `replicated`, which binds Immediate
either way. Both branches are pinned in tests, including the
existing-DataVolume path, which renders its own annotation block.
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
Assisted-By: LLM
Requesting immediate binding for every source, not just `upload`, is
deliberate: a VMDisk is a standalone object populated before any VM
consumes it, so on a WaitForFirstConsumer StorageClass a disk with no VM
attached would never populate at all.
What that costs was recorded nowhere an operator reads. On a node-pinned
class the CDI worker pod chooses the node, so the volume binds where the
importer landed rather than where the VM will run, and a VM constrained
elsewhere -- a nodeSelector, a request only one node satisfies, or the
Windows affinity from _cluster.scheduling.dedicatedNodesForWindowsVMs --
stays unschedulable against a node-affinity conflict. One disk is enough
for that; it is not a multi-disk-only case. storageClass is immutable,
so the exit is delete and recreate. The chart default `replicated` binds
Immediate regardless and does not move.
Also pinned in tests, in both directions the annotation is emitted: per
source type (blank, image, disk) so a regression to the old upload-only
condition reddens, and on the existing-DataVolume path, where `helm
upgrade` retrofits the annotation onto disks created before it was
unconditional -- inert for a volume that has already bound, and now a
recorded decision rather than an unasserted side effect.
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
Assisted-By: LLM
A VMDisk is a standalone object that is populated before any VM consumes
it, so on a WaitForFirstConsumer StorageClass CDI needs an explicit
signal to schedule its worker and start provisioning. Without it the
DataVolume sits in PendingPopulation indefinitely and the disk is never
filled -- and nothing says so.
This is a consistency fix rather than a new behaviour. The chart default
`replicated` is volumeBindingMode: Immediate, so a disk on it already
bound and populated at creation; `local`, the cluster's default class,
is WaitForFirstConsumer, so the same VMDisk silently did nothing there.
The annotation was previously emitted only for source: upload, which is
the one source that could not work without it, leaving http, image, disk
and blank to differ by class.
Two consequences worth knowing. CDI checks the annotation by presence
and ignores its value, so "" and "true" are equivalent and this is not a
behaviour change on the upload path. And on a node-pinned class each
disk now binds at its own creation rather than at VM scheduling, so a
multi-disk VM on `local` can find its disks on different nodes -- that
is the existing limitation of a node-pinned class, which is why the
chart default is not one, but it is reached earlier now.
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
Editing a VMDisk's storageClass or source after its DataVolume already
exists was silently ignored: dv.yaml reused the existing DataVolume.spec
verbatim (DataVolume.spec is immutable in CDI), so the edit rendered as a
no-op while the HelmRelease reported success and the CR diverged from the
cluster.
When a DataVolume already exists, fail with a clear message if the
requested storageClass or source differs from what the disk was created
with. Both compare the operator's input recorded in an annotation at
creation (vm-disk.cozystack.io/storage-class and .../source), written only
at creation and preserved verbatim afterwards — never overwritten with an
unapplied edit. Recording the input rather than reading the live object
matters on both axes: the image-clone PVC name carries a chart-version
prefix (vm-image- was renamed to vm-default-images-, commit bec35e3a) that
drifts across upgrades with no edit, and the live PVC storageClassName of an
adopted or foreign DataVolume may legitimately differ from the CR default
without being a change. The annotation key presence (not its value) marks a recorded class, so a disk
created with the cluster default "" is recorded and still caught if later
changed to a concrete class.
Disks with no annotation (created earlier, or out-of-band) are handled
conservatively so a working release never deadlocks: source is compared
against the stored spec.source only when that stored source is a shape the
chart itself produces, so an adopted DataVolume using spec.sourceRef or a
non-chart source (registry, snapshot, http+secretRef, ...) is left alone
rather than deadlocked. The image-clone exemption fires only when the stored
source is a cozy-public pvc AND the request is itself an image, so a change
away from an image source to another type is still caught. storageClass has
no safe live fallback, so an un-annotated disk is not guarded here (apiserver
CEL, #2657, is the eventual guard). An unrecognized source type
now fails explicitly. Storage growth is unaffected and still handled by
pvc-resize-hook.yaml.
Mark source as immutable in values.yaml so the generated schema carries
the same x-kubernetes-validations rule already present on storageClass,
and document the contract, the exemption, and the source-annotation schema
coupling in docs/storage-immutability.md and the chart README.
Add helm-unittest coverage for the immutability guard (rejection incl.
clearing storageClass to empty and changing a non-image disk to image,
no-op reuse with annotations preserved, legacy image-disk drift, annotation
non-poisoning, un-annotated storageClass skip, adopted default-class disk,
unknown source) and initial-creation source rendering.
Fixes#2985
Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io>
The vm-disk pvc-resize, cozystack-api, seaweedfs and vm-instance update
hooks (whose alpine/k8s image this PR refreshes) ran as root with no
securityContext. Add a pod-level runAsNonRoot + runAsUser 65534 +
seccompProfile RuntimeDefault and a container-level
allowPrivilegeEscalation: false + capabilities drop ALL, mirroring the
dashboard adopt hook. kubectl needs no root, so this is behavior-neutral.
Each hook is lookup-gated; pin the securityContext in a per-chart
helm-unittest that mocks the gating object (DaemonSet / PVC / Service) via
kubernetesProvider so the guard renders without a cluster. vm-disk and
vm-instance had no Makefile test target, so add 'test: helm unittest .' to
both so CI runs the new suites.
Scope note: other kubectl hooks on clastix/kubectl (mariadb, keycloak,
kubernetes, tenant) are untouched here — they are not part of this
alpine/k8s refresh and belong to a separate hardening pass.
Also fix the vm-disk resize hook's backoffLimit: it was nested under the
pod spec (silently ignored, leaving the Job at the default 6 retries)
instead of Job.spec. Move it to Job.spec and pin it in the test.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
Myasnikov Daniil