mirror of
https://github.com/cozystack/cozystack.git
synced 2026-11-06 03:57:52 +09:00
Aleksei Sviridkin
15035a915b
## 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 -->