mirror of
https://github.com/cozystack/cozystack.git
synced 2026-11-03 10:37:21 +09:00
Aleksei Sviridkin
fbf69f7aad
## 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 -->
Backport adaptation: release-1.6 has no server-side-apply release
annotation, so TestReleaseAnnotationsShareThePrefix lists the three
annotations this line has, main's TestParseHelmServerSideApplyAnnotation
is not brought in, and the comments count three constants and name
CozyServerOptions.Complete, where this line reads the annotations.
(cherry picked from commit 15035a915b)
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
Assisted-by: LLM
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>