fix(vm-instance): disable helm readiness wait on the VMInstance release - #3735

IvanHunters NOT LGTM, for one missing test. The premise holds at the versions this repo ships and disabling the wait is the right fix, but the new branch that skips an invalid annotation can be deleted without any test going red.

Business context: since helm-controller v1.5.0 a VMInstance release waits on a KubeVirt VirtualMachine that kstatus often cannot see as ready, so healthy VMs loop upgrade and timeout. This PR turns the wait off for that kind and carries the setting to releases that already exist.

Premise at the pinned versions

The repo ships helm-controller v1.5.0, which pins Helm v4.1.1, fluxcd/cli-utils v0.37.1-flux.1 and fluxcd/pkg/runtime v0.100.2, plus KubeVirt v1.9.0. With neither DisableWait nor a waitStrategy set, getWaitStrategy in helm-controller falls through to the kstatus poller. KubeVirt v1.9.0 writes status.observedGeneration only from syncGenerationInfo, which runs only while a VMI exists. The generation it copies is bumped only when the template matches the controller revision (vm.go:1438-1465). So a stopped VM, a restart-pending change or a live update all leave observedGeneration behind, and the hang is real.

One correction on the mechanism, which is also in the YAML comment and in #3734. vm-instance sets no healthCheckExprs, so the CEL StatusEvaluator never runs for its VirtualMachine. What fires is kstatus's own checkGeneration in cli-utils (pkg/kstatus/status/generic.go), which has the same rule. Past that check, Compute falls back to checkReadyCondition, so a halted VM with Ready=False is InProgress even when its generation is in sync. This does not change the conclusion. If anything it makes disable-wait a better fit than healthCheckExprs.

What loses its readiness signal

The HelmRelease Ready, and the Application Ready that mirrors it (rest.go keeps WorkloadsReady out of Ready on purpose). On main it meant the VirtualMachine is Ready. After this PR it means the manifests are applied and the update hook Job finished, since hooks are still waited on. That matches the behaviour before v1.5.0, when Helm 3's checker treated any custom resource as ready.

VM health stays visible in WorkloadsReady. The chart's WorkloadMonitor selects the virt-launcher pod by app.kubernetes.io/instance, and for external VMs it also wants LoadBalancer ingress on the Service. I found nothing that waits on a VMInstance HelmRelease: no dependsOn points at one, the Velero strategy and the VM import controller never read its Ready condition, and the vminstance chainsaw suite checks the VMI IP and VM Ready on their own after the HelmRelease assert, so it still covers the boot.

Blocker: the invalid-annotation branch has no test

internal/controller/applicationdefinition_helmreconciler.go:179. The reconciler leaves DisableWait alone when the annotation does not parse. I changed if err != nil { to if err != nil && false { and every test in ./internal/controller still passed. In that state a value like "yes" parses to false and sets DisableWait=false on every release of the kind, which is what the comment says the branch prevents. A test that starts from a release with DisableWait=true, sets the annotation to "yes" and asserts the release is unchanged would close it.

Non-blocking

1. The body says waitStrategy: legacy "buys nothing here, as those are ready the moment they are applied". That is not true for external: true: Helm's legacy checker keeps a LoadBalancer Service not ready until it has ingress (pkg/kube/ready.go:260-270 at v4.1.1), so legacy would still gate external VMs on IP allocation. Disable-wait is still the better choice, because the kstatus Service check only looks at clusterIP and WorkloadsReady does watch ingress. The body becomes the merge commit, so please fix that sentence and the CEL attribution above.
2. Both controllers touched here run in cozystack-controller (cmd/cozystack-controller/main.go:255,263), not in cozystack-operator, which is a separate binary. The new ReleaseAnnotationPrefix comment says cozystack-operator rolls the api, the last two commit subjects use fix(operator), and the body calls these "operator-side gaps".
3. The label selector that keeps the carry-over on its own kind has no test. Dropping apps.cozystack.io/application.kind from it leaves the suite green. That gap is older than this PR, but the reconciler now writes the wait too, so one release of another kind in the new tests would pin it.
4. The 20-line comment on the annotation repeats the PR body and the header of readiness_gate_test.yaml. Two lines on why (observedGeneration trails generation, so kstatus never reports the VM Current) would be enough there.
5. cozystack-api reads the annotation only at start-up, and the carry-over reacts only to ApplicationDefinition events. An Application written through an api pod that has not restarted yet goes back to DisableWait=false until the definition is reconciled again. The window is short.

Tests: helm unittest on vm-instance-rd passes 2/2, and go test ./internal/controller ./pkg/config passes 54 top-level tests. I mutated the annotation value, the hash field, the prefix filter, the block creation, the upgrade assignment and the in-sync check, and each one turned the matching test red. Only the invalid-annotation branch and the kind selector survived.

CI: the E2E run on this head (35774391142) is still running and I have not seen its suites. The previous run on the earlier head with the same three commits (35196009993) failed in kafka (context deadline) and opensearch (StatefulSet currentRevision != updateRevision), while vminstance and vmdisk passed. Neither failure touches this diff: those definitions carry no disable-wait annotation and their releases already have DisableWait=false, so the carry-over does nothing to them.

IvanHunters LGTM. The branch that skips an unparseable annotation is pinned now. I changed if err != nil { to if err != nil && false { and TestAppDefHelm_DisableWaitInvalidAnnotationLeavesReleaseAlone went red, with nothing else failing. Dropping the kind label from the selector turns TestAppDefHelm_DisableWaitScopedToKind red, so the kind scope is covered as well.

The base did not move, and the rework is the two tests plus comment, body and subject fixes. The body now puts the hang on kstatus checkGeneration and says why waitStrategy: legacy would still hold an external VM on its LoadBalancer. go test ./internal/controller ./pkg/config passes 56 top-level tests and helm unittest on vm-instance-rd passes 2/2. CI on this head is green, E2E included.

One optional leftover. The first commit's message still says the generation check is "a built-in pre-check in fluxcd/pkg/runtime/cel" that healthCheckExprs cannot override. Since vm-instance sets no healthCheckExprs, the check that fires is kstatus checkGeneration, as the body says now. This repo merges with merge commits, so that sentence stays in history unless you reword it.