fix(virtual-machine): vminstance disk race + bump VM IP/ready timeouts - #2555

hack/e2e-apps/vminstance.bats (1)

83-84: ⚡ Quick win

Move end-of-test cleanup to a teardown() function and add --ignore-not-found.

bats' teardown() runs after each test regardless of whether it passed or failed. As these delete calls are in the test body, any failure between kubectl apply (line 42) and here (e.g., the 120 s IP-assignment timeout on line 79 expiring) leaves the VMInstance and VMDisk in the cluster. The next cleanup only covers vminstances (line 41); the vmdisk survives until "Create a VM Disk" runs, meaning a standalone re-run of just this test accumulates leaked disks.

Additionally, lines 83-84 lack --ignore-not-found. If either resource is gone by the time cleanup runs, the delete fails and the test is reported as failed despite having exercised its actual assertions successfully.

♻️ Proposed refactor: move cleanup to `teardown()`

+teardown() {
+  kubectl -n tenant-test delete vminstances.apps.cozystack.io test --ignore-not-found --timeout=3m || true
+  kubectl -n tenant-test delete vmdisks.apps.cozystack.io test --ignore-not-found --timeout=3m || true
+}
+
 `@test` "Create a VM Disk" {

-  kubectl -n tenant-test delete vminstances.apps.cozystack.io $name --timeout=3m
-  kubectl -n tenant-test delete vmdisks.apps.cozystack.io $diskName --timeout=3m
 }

The || true in teardown() is intentional: a failed cleanup must not mask a passing test, and repeated teardown calls on already-deleted resources should not block the next test. The pre-run deletes at lines 12-13 and 41 can remain as a defensive "flush stale state before setup" guard, or be folded into setup() for the same reason.

Based on learnings: the codebase convention for E2E bats tests is to replace inline cleanup with framework-level teardown()/teardown_file() rather than embedding kubectl delete in the test body.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@hack/e2e-apps/vminstance.bats` around lines 83 - 84, Move the two inline
cleanup kubectl delete calls for vminstances.apps.cozystack.io and
vmdisks.apps.cozystack.io out of the test body and into a bats teardown()
function so they always run after each test; in teardown() call kubectl delete
for $name and $diskName with the --ignore-not-found flag and ensure the commands
are resilient (e.g., append || true) so cleanup failures or already-removed
resources don't cause test failures, and remove the original delete lines from
the test body.

hack/e2e-apps/vminstance.bats (1)

12-13: 🏗️ Heavy lift

Move inline cleanup to framework-level teardown for failure-safe cleanup.

Cleanup is still embedded in test bodies, so failures before the final delete path can leave residue. Please migrate these deletes to Bats teardown/global cleanup to guarantee execution regardless of where a test aborts.

Based on learnings: “In E2E bats tests … replace inline cleanup … with framework-level teardown/cleanup … apply this pattern to all bats tests under hack/e2e-apps.”

Also applies to: 41-42, 83-84

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@hack/e2e-apps/vminstance.bats` around lines 12 - 13, The two inline kubectl
cleanup commands should be moved into a framework-level teardown to guarantee
execution on test failure: remove the inline deletes from test bodies and
implement a teardown_file (or teardown) function in vminstance.bats that
iterates over a shared list/array of resource names (e.g., resource_names or
created_vms) and runs kubectl -n tenant-test delete
vminstances.apps.cozystack.io <name> and kubectl -n tenant-test delete
vmdisks.apps.cozystack.io <name> with --ignore-not-found --timeout=3m; update
the test resource-creation code (places where name is assigned) to append the
name to that shared list so the teardown can clean them up, and apply the same
refactor for the other inline deletes referenced (lines 41-42 and 83-84).

hack/e2e-apps/vminstance.bats (1)

86-87: ⚡ Quick win

Cleanup leak path: move final deletes to a teardown() function

If the IP-poll (line 82), HR-ready (line 83), or VM-ready (line 85) assertion times out, bats exits the test immediately and lines 86–87 never execute, leaving vminstances and vmdisks resources live in the cluster. A subsequent standalone or re-run invocation will then hit the pre-clean on lines 43–44 and wait up to 3 min each for those orphaned resources to finalise.

Moving cleanup into a teardown() guarantees it runs regardless of how the test exits.

♻️ Proposed refactor: `teardown()` function

+teardown() {
+  kubectl -n tenant-test delete vminstances.apps.cozystack.io test --ignore-not-found --timeout=3m || true
+  kubectl -n tenant-test delete vmdisks.apps.cozystack.io test --ignore-not-found --timeout=3m || true
+}
+
 `@test` "Create a VM Disk" {

-  kubectl -n tenant-test delete vminstances.apps.cozystack.io $name --ignore-not-found --timeout=3m
-  kubectl -n tenant-test delete vmdisks.apps.cozystack.io $diskName --ignore-not-found --timeout=3m
 }

Note: teardown() must suppress errors (|| true) because bats marks the test as failed if teardown itself exits non-zero and the resource may already be gone.

Based on learnings: "In E2E bats tests like hack/e2e-apps/harbor.bats, replace inline cleanup (e.g., kubectl delete in the test body) with framework-level teardown/cleanup… Apply this pattern to all bats tests under hack/e2e-apps."

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@hack/e2e-apps/vminstance.bats` around lines 86 - 87, Move the two inline
cleanup kubectl delete calls that remove vminstances and vmdisks (the commands
using $name and $diskName) into a bats teardown() function so cleanup always
runs even if earlier assertions time out; implement teardown() to run kubectl -n
tenant-test delete vminstances.apps.cozystack.io $name --ignore-not-found
--timeout=3m and kubectl -n tenant-test delete vmdisks.apps.cozystack.io
$diskName --ignore-not-found --timeout=3m, ensure each delete command suppresses
errors (append || true) and remove the original inline deletes from the test
body so only teardown handles final deletion.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes