Keying the network-data on instanceProfile reached every VM that never
set the field, because the profile defaults to ubuntu whatever image
the disk holds. The sysconfig, NetworkManager and eni renderers turn
each match stanza into an interface literally named after the netplan
ID, so a non-netplan guest left on the default profile would lose the
DHCP it gets today. A VM with neither cloudInit nor sshKeys would also
gain a cloud-init disk on upgrade, and KubeVirt's NoCloud instance-id
is new to that guest, so its next boot would regenerate host keys as a
first boot.
The configuration also shadows a static NIC set up through netplan:
netplan emits one networkd file per ID, networkd applies only the first
file in lexical order that matches a link, and 10-netplan-cozy-en sorts
before the file for enp2s0. The external-VLAN recipe claimed otherwise.
An explicit netplanDhcp value, off by default, leaves every existing
VM rendering exactly as before and lets the documentation say when not
to turn it on.
Assisted-by: LLM
Signed-off-by: Mattia Eleuteri <mattia@hidora.io>
The memory guard refused only zero and negative amounts, while KubeVirt
admission refuses a non-zero amount below 1M. A value such as 500m or
1Ki rendered cleanly and then failed when the VirtualMachine was
applied, two hops away from the value that caused it. The render now
refuses it with the reason.
This is breaking: a memory above zero and below 1M rendered before and
no longer does. Such a VM could not be admitted by KubeVirt either.
Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
The cpu and sockets descriptions named millicore and fractional values
as refused, while the guard also refuses other unit suffixes, zero,
negative, past-uint32 and quoted leading-zero values. An operator
reading the schema got a narrower rule than the render enforces. The
error message gains the negative case for the same reason.
Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
## What this PR does
A VMInstance is served as a Flux HelmRelease that renders a KubeVirt
VirtualMachine. Since v1.5.0 (helm-controller v1.4.3 to v1.5.0) that
release is health-assessed by the kstatus poller. vm-instance sets no
`healthCheckExprs`, so the VirtualMachine goes through kstatus's own
rules in `fluxcd/cli-utils` (`pkg/kstatus/status`): `checkGeneration`
returns InProgress for any object whose `status.observedGeneration`
differs from `metadata.generation`, and past that check a `Ready=False`
condition is InProgress too, so a halted VM is never Current even with
its generation in sync. KubeVirt keeps a VirtualMachine's
`observedGeneration` behind its `generation` whenever a `spec.template`
change is pending a restart, the VM is stopped, or a live-updatable
change never converges (kubevirt/kubevirt#18700). So a healthy, Running
VM is read as permanently InProgress, the upgrade waits its full
timeout, and fails with `timeout waiting for: [VirtualMachine/...
status: 'InProgress']`. On releases still carrying the pre-1.5.0
`remediation.retries: -1` + rollback spec, the timeout then reverts the
previous manifest, dropping just-applied changes such as opened ports.
This sets `release.cozystack.io/helm-install-disable-wait: "true"` on
the vm-instance ApplicationDefinition, so cozystack-api generates the
HelmRelease with Install/Upgrade DisableWait. The release settles once
the VirtualMachine is applied, and VM health is read off the
VirtualMachine itself (`status.printableStatus` / `Ready`) and off the
WorkloadMonitor the chart already renders, which is what
`WorkloadsReady` on the Application reports. This restores the pre-1.5.0
behaviour, under which the VM was never a helm readiness gate. The same
mechanism is already used by `kubernetes-rd` and `kubernetes-nodes-rd`.
On its own that annotation does not reach a running cluster, so two gaps
in cozystack-controller are closed with it.
The first is the rollout. `applicationdefinition-controller` hashed each
definition's name and spec only, so a change confined to a
`release.cozystack.io/*` annotation left the hash unmoved and
cozystack-api kept serving the configuration it read at its last
start-up, which is the only place those annotations are ever read. The
hash now covers them too. Only that prefix, because kubectl and Helm
rewrite annotations of their own on every apply and hashing those would
roll the api Deployment on every reconcile.
The second is the releases that already exist.
`applicationdefinition-helm-reconciler` brings chartRef, valuesFrom and
labels on existing HelmReleases in line with their definition; it now
does the same for the readiness wait. Without it the annotation reaches
new instances only, which on an affected cluster is the wrong half:
#3734 reports 134 of 136 releases already looping upgrade and timeout,
and re-applying every VMInstance by hand was the only way out. The
setting is driven in both directions, since cozystack-api rebuilds the
whole spec on every write and emits `DisableWait=false` once the
annotation is gone. An absent install or upgrade block already behaves
as `DisableWait=false` and is materialised only when the definition asks
to disable the wait, so a release with nothing to change is not
rewritten.
Two alternatives were considered. A per-kind `healthCheckExprs` keyed on
`status.printableStatus` cannot work: setting it moves the
VirtualMachine from kstatus to the CEL status evaluator
(`fluxcd/pkg/runtime/cel`), which runs the same `observedGeneration`
check ahead of the expressions, and any `healthCheckExprs` added to this
kind later is a no-op while the wait is disabled. The typed
`spec.release.waitStrategy: legacy` is the narrower option, since the
legacy checker never waits on a custom resource, but it still gates the
release on the chart's Service, and with `external: true` that is a
LoadBalancer the legacy checker holds not ready until it has ingress. So
an external VM would still wait on IP allocation, which `WorkloadsReady`
already watches. Disable-wait avoids that and is the mechanism this
repository already uses for the same class of problem.
Tests: a helm-unittest suite (`tests/readiness_gate_test.yaml`) guards
that the annotation stays on the rendered ApplicationDefinition, and Go
unit tests cover both cozystack-controller paths: the hash moves on a
release annotation and stands still on an unrelated one, and the wait
lands on an existing release, is withdrawn when the annotation goes,
does not materialise empty install/upgrade blocks, does not rewrite a
release already in sync, leaves a release alone when the annotation does
not parse, and does not reach a release of another kind.
Note on existing clusters: no manual step. The definition changes on
upgrade, the config-hash rolls cozystack-api, and the helm reconciler
brings existing VMInstance releases in line. Releases that predate
v1.5.0 also carry a `remediation.retries: -1` + rollback spec, which is
left alone: with the wait disabled the upgrade no longer times out, so
that remediation no longer fires, and any write through the apps API
drops it.
Closes#3734.
### Downstream repositories
- [x] No downstream repository is affected by this change
<!-- Walked the trigger map against the diff. The change is a readiness
annotation on an ApplicationDefinition plus two cozystack-controller
paths. It does not touch openAPISchema, kind, plural, release.prefix,
any app package, any values.schema.json or any installer/platform values
key. The website's application-definitions.md describes the config-hash
as a checksum "over the sorted definitions", which stays accurate now
that the hash also covers the release.cozystack.io/* annotations, and no
page documents those annotations today. -->
### Release note
```release-note
fix(vm-instance): VMInstance HelmReleases no longer hang InProgress or roll back healthy VMs; the release no longer waits on VirtualMachine readiness, which helm-controller's kstatus poller cannot assess correctly for KubeVirt VMs. Existing VMInstance releases are brought in line automatically on upgrade.
```
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **Bug Fixes**
* VM installations no longer wait indefinitely when Kubernetes reports
the VM’s observed generation behind its current generation.
* Changes to release settings are now detected and applied to generated
Helm releases, including install and upgrade wait behavior.
* **Tests**
* Added coverage for VM readiness and release-setting reconciliation.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
The annotation comment repeated the PR description and the test header;
two lines on why are enough. The hang comes from the kstatus generation
check in cli-utils, not the CEL status evaluator, since vm-instance sets
no healthCheckExprs. The config-hash rollout runs in cozystack-controller,
not cozystack-operator.
Assisted-by: LLM
Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
resources.cpu and resources.sockets are typed as quantities, but they
land in domain.cpu.cores and domain.cpu.sockets, which KubeVirt
declares as uint32. The chart cast them with int64, which turns 500m,
1.5 or 1e1 into 0 without an error, and KubeVirt boots a zero core
count as one core. The user asked for half a core and got a one-core
VM with nothing reporting it.
The raw spelling is now decoded as YAML, the way an integer field
reads it, and the render stops unless the result is a positive whole
number that fits in uint32. A leading zero is refused, because YAML
reads 010 as octal 8. The check keys on presence rather than
truthiness, so an unquoted 0 is refused instead of being read as
unset.
Memory gets the same presence check. A quoted "0" was truthy and
rendered requests.memory: "0", while an unquoted 0 was falsy and
dropped, so one quantity took two paths. A zero or negative amount
now stops the render in both spellings.
BREAKING CHANGE: values that used to render now either fail or
render differently. A cpu or sockets that does not decode to a plain
whole number within uint32 (a millicore, suffixed such as 1k,
fractional, zero, negative or leading-zero value), and a zero or
negative memory, fail the next upgrade of the release until the
value is fixed. This includes cpu or sockets set on its own beside an
instanceType, which used to be ignored whatever it held. A spelling
that the int64 cast truncated to 0 but that YAML decodes to a whole
number, such as the exponent 1e1 or the trailing dot 2., used to boot
one core and now sizes the VM with the count it spells, so such a VM
changes size on upgrade.
Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
The generated HelmRelease is health-assessed by helm-controller's kstatus
poller, which returns InProgress for any object whose
status.observedGeneration trails metadata.generation: a built-in pre-check
in fluxcd/pkg/runtime/cel that runs before, and cannot be overridden by,
healthCheckExprs. KubeVirt legitimately keeps a VirtualMachine's
observedGeneration behind its generation while a template change is pending
a restart, the VM is stopped, or a live-updatable change never converges
(kubevirt/kubevirt#18700), so a healthy Running VM is read as permanently
InProgress and the release never reaches Ready. Releases still carrying the
pre-1.5.0 remediation.retries:-1 + rollback spec then revert the previous
manifest, dropping just-applied changes such as opened ports.
Disable the readiness wait so the release settles once the VirtualMachine is
applied; VM health is observed on the VirtualMachine itself. This restores
the pre-1.5.0 behaviour under which the VM was never a helm readiness gate.
A helm-unittest suite guards that the annotation stays in place.
Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
KubeVirt refuses a VirtualMachine that carries an instancetype matcher and
explicit domain sizing at once. validateCPU in pkg/instancetype/apply
conflicts on domain.cpu.cores and domain.cpu.sockets when they are
non-zero, and validateMemory conflicts on
domain.resources.requests.memory whenever the key is present. The chart
rendered both, because the matcher keyed off instanceType while the
domain sizing keyed off resources, and neither side looked at the other.
values.yaml ships instanceType "u1.medium", so clearing that field in the
dashboard only falls back to the default. A user who then fills in cpu,
sockets and memory gets a VM the webhook rejects, with an error naming
fields the form gives them no way to clear.
Resources that size the VM completely now replace the matcher, the same
way the kubernetes-nodes chart sizes its worker VMs. Beside an instance
type, sizing that is only partial stops the render, because there is then
no safe reading: dropping the matcher would size the VM from fields the
user never finished, and keeping it either loses to the webhook or
silently discards what was typed. With no instance type in play there is
nothing to conflict with, so that half is left as it renders today.
Completeness is decided from what domainResources emitted, not from which
keys the user set, and the two readings come apart. resources.cpu on its
own emits nothing, so it conflicts with nothing and goes on rendering a
plain instancetype-sized VM. cpu and sockets pass through int64, which
yields 0 for a quantity the schema accepts, so a cpu of 500m beside a
whole sockets and a memory would read as complete from the values, drop
the matcher, and hand KubeVirt a cores of 0 the user never asked for. And
a resources block nulled outright leaves .Values.resources nil, where a
raw field read is a render-time nil pointer and the emitted object is
merely empty.
The existence check and the update hook read the same effective value.
Keying the lookup on the raw one would fail a resource-sized render over
an instance type it never references, which is the same confusing error
this fixes. Reading .Values in the hook would see the leftover
instanceType, skip the removal patch, and leave
spec.instancetype.revisionName on a VM Helm had just given explicit
domain resources.
Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
Hidora's customers are leaving VMware now, and a tenant has to be able to
import their own machines without a platform administrator. The chart
approach could not express that: a tenant can neither create the Secret
holding vCenter credentials nor name the VDDK image, so both had to come
from somewhere they cannot reach.
Two CRDs in a new `forklift.cozystack.io` group, split by lifecycle. A
VMImportSource is a long-lived connection -- type, endpoint, credentials,
readiness -- and a VMImportTask is the one-shot operation that names
machines and produces VMDisks and VMInstances. The outputs carry no owner
reference back to the task, so deleting a finished import leaves the
machines alone. Konveyor Forklift does the transfer underneath; the
controller renders its objects, mirrors its verdicts, and hands each
finished volume into a VMDisk without copying it.
What a live vCenter migration forced out, and no amount of source reading
would have:
- A guest copied verbatim keeps the drivers it had under VMware and no
virtio, so on a virtio disk it does not fail the import -- it succeeds
and then does not boot. Imported disks go on SATA.
- VDDK connects straight to the ESXi host at the address vCenter
advertises, which is routinely unreachable, and on the first cluster
collided with the Service CIDR. `spec.hosts` redirects per host.
- Firmware must be carried across or a UEFI guest imports as BIOS and
never boots (stacked on #3002).
- Deleting a task mid-transfer orphaned its DataVolume; a finalizer now
removes the engine's volumes and leaves the outputs.
The console gains a Migration section: connections are editable, and the
VM field is a picker built from a machine list the controller publishes,
since the aggregated API cannot reach the inventory and should not hold a
source's credentials to try. Two general mechanisms fall out of that --
an option source may take an argument, and one named in a CRD annotation
may reference a sibling field.
Design: cozystack/community#62. Docs: cozystack/website#673.
Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
Regenerate values.schema.json and embedded openAPISchema in the -rd
ApplicationDefinitions so the x-cozystack-options dynamic-source
directives declared in each app values.yaml are carried into the
generated artifacts. Brings the -rd schemas in sync with the
hand-edited values.schema.json files (CI runs make generate +
git diff --exit-code).
Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Andrei Kvapil <andrei.kvapil@aenix.io>
Address review feedback from lexfrei: dropping the centos.7/centos.7.desktop/
centos.stream8{,.desktop,.dpdk} VirtualMachineClusterPreference objects hard-fails
the vm-instance Helm render (vm.yaml fails when a referenced preference is absent)
and makes the dynamic CRD enum reject the stored value for every existing VM on
those profiles on upgrade. Re-add them as deprecated aliases in
extra-instancetypes.yaml, mirroring the gn1.* GPU instancetype treatment, and
extend the vm-instance enum generation to union preference names from
extra-instancetypes.yaml so the retained profiles stay in values.schema.json and
the RD schema.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
`make update` stripped `persistent: true` from upstream's
`preferredTPM: { persistent: true }`, leaving the parent key with a null value.
KubeVirt v1.6's CRD tolerated it; v1.8 (the operator version we bumped to in
6af0bf71) enforces structural schema and rejects null:
VirtualMachineClusterPreference "windows.11" is invalid:
spec.devices.preferredTPM: Invalid value: "null": spec.devices.preferredTPM
in body must be of type object: "null"
Stop stripping the line and regenerate the templates from upstream
common-instancetypes. Persistent TPM state falls back to the cluster default
StorageClass with RWO access mode when `vmStateStorageClass` is unset on the
KubeVirt CR, per pkg/storage/backend-storage/backend-storage.go.
Drive-by from the regen:
- Removed (upstream EOL): centos.7*, centos.stream8*
- Added: debian, oraclelinux, legacy, linux*, fedora.s390x, rhel.{9,10}.s390x,
windows.{xp,7,7.virtio,2k3,2k8,2k8.virtio,2k12,2k12.virtio}, plus new
cx1.*1gi and d1 instancetype variants.
Also regenerates packages/apps/vm-instance instanceProfile enum
(values.schema.json / README.md) from the refreshed preferences.yaml.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
- Add `externalAllowICMP` value (default true) propagated as
`networking.cozystack.io/allowICMP` annotation on the rendered Service
when `externalMethod: PortList`. The cozy-proxy companion (released as
part of cozystack/cozy-proxy#11 + #12) drops ICMP by default in
port-filter mode, which breaks ping and PMTU discovery; defaulting the
chart to "true" preserves user expectations while still allowing
operators to opt out by setting `externalAllowICMP: false`.
- Remove the v1.3.1.md changelog entry. Project convention is to add
changelogs in a dedicated "docs: add changelog for vX.Y.Z" commit at
release time, not as part of feature/fix PRs.
Signed-off-by: mattia-eleuteri <mattia@hidora.io>
Port three features from virtual-machine to vm-instance:
- Add cpuModel field to specify CPU model without instanceType
- Allow switching between instancetype and custom resources via update hook
- Migrate from running boolean to runStrategy enum
Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
The apps.cozystack.io/application.kind, apps.cozystack.io/application.group,
and apps.cozystack.io/application.name labels already exist on all HelmReleases
and provide more specific identification. Remove the redundant cozystack.io/ui
label and update all selectors to use apps.cozystack.io/application.kind instead.
Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
Rename the CRD and all related types for better clarity:
- CozystackResourceDefinition -> ApplicationDefinition
- CozystackResourceDefinitionList -> ApplicationDefinitionList
- CozystackResourceDefinitionSpec -> ApplicationDefinitionSpec
- All related nested types updated accordingly
Updated components:
- API types and generated deepcopy code
- Controllers and reconcilers
- Dashboard, lineagecontrollerwebhook, crdmem packages
- CRD YAML definition and Helm chart
- All 25 cozyrds YAML manifests
- Migration scripts and documentation
Added migration 23 to remove old cozystack-resource-definition-crd HelmRelease.
Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
Move common-envs.mk and package.mk from scripts/ to hack/ directory.
Update all Makefile includes to use new paths. Remove unused
issue-flux-certificates.sh script.
Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
Update all CozystackResourceDefinition files to use chartRef
with ExternalArtifact instead of OCIRepository sourceRef.
Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
Mattia Eleuteri