fix(e2e): disable guest fsync on ephemeral CI VM disks - #3455

Actionable comments posted: 1

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

Inline comments:
In `@hack/e2e-chainsaw/_lib/run-kubernetes.sh`:
- Around line 444-467: The fixed 18m node-readiness wait can exceed the
enclosing 40m Chainsaw step after earlier waits consume most of the budget.
Update the node-join polling block and its failure message to use a shared or
remaining timeout budget calculated after the preceding waits, or increase the
step timeout to cover the declared maximums and downstream checks.

E2E is red on this head, and on the two suites this change exists to stabilise, so I have not reviewed the diff yet.

In run 30274169767 both kubernetes-latest and kubernetes-previous failed with fewer than 2 tenant nodes Ready within 18m. What that run captured points away from both halves of the change: the worker DataVolumes reached Phase: Succeeded at Progress: 100.0%, both worker VMIs were Running with Ready true and an address assigned, and the (c) tenant CSRs capture printed No resources found in default namespace. in each suite — no kubelet CSR at all across the full 18 minutes. The guests booted and the widened budget was not the limiting factor; the tenant cilium HelmRelease was still mid-install because no Node had registered.

None of that says the change is wrong. It says this run does not demonstrate the effect the description claims, and it does not support the rationale the diff writes into the code, which attributes the overrun to host storage pressure consuming the margin.

The ask: land a green run on this head, or — if it keeps failing this same way — point the description and the in-code rationale at what the runs actually show, then re-request review.

NOT LGTM — the diff itself is harmless and both halves are defensible, but the rationale it writes into the code names a cause the run on this very head contradicts, and that comment will outlive the PR.

Business context: Stop the VM-heavy chainsaw suites from failing a different case each run by removing the host fsync path that stalls the management etcd, and give tenant worker bring-up a single budget wide enough to absorb it.

Blockers

B1: the 18m rationale attributes the overrun to a margin that the run on this head shows was not the limit

File: hack/e2e-chainsaw/_lib/run-kubernetes.sh:447

Issue: the new comment explains the widening as "Under host storage pressure that margin evaporates … the worker VMIs were created ~2m into this wait, and the guests then had only ~10m to import Talos, boot, register a kubelet and let cilium turn the nodes Ready. They did not make it", concluding "this is load-induced slowness, not a stuck bring-up; 18m restores margin". The run on this head does not support that reading, and it is the run a future reader will check first.

Evidence: run 30274169767 (head 18c78e1, both changes in effect) failed both suites with fewer than 2 tenant nodes Ready within 18m. In that run the worker DataVolumes reached Phase: Succeeded at Progress: 100.0% — the image import, the part host storage pressure would slow, completed. Both worker VMIs were Running with Ready true and an address, so the guests booted with roughly 15 minutes left on the clock. The (c) tenant CSRs capture printed No resources found in default namespace. for each suite: not a late CSR, no CSR at all in 18 minutes. A budget that the guest never gets close to spending is not the quantity that failed. The comment's own reference run points the same way: it describes 30260770694 ending with "zero Nodes registered and the tenant cilium HR still mid-install", which is exactly how 30274169767 ended — with the import finished on time. The same terminal state on both sides of the import delay means the import delay is not what produces it.

Impact: the comment reads as "cause identified, margin restored", so the next person to meet this failure has a documented reason to widen the budget again rather than look at why the kubelet never submits a CSR. That is the same loop the no-retry convention in docs/agents/e2e-testing.md exists to break — a budget increase is not a retry, but it hides a deterministic failure the same way.

Fix: keep the widening, and rewrite its justification around what is measured rather than around the margin: the two terms are CSR issuance and CSR→Ready, and the second one alone has been observed above five minutes, which is what makes 12m structurally tight. Then drop the claim that this removes the flakiness — on this head it does not — and say what it does: it stops the budget from being the first thing to fire, so the next failure lands on the real term.

Non-blocking follow-ups

1. The widened deadline also applies on the way to a failure, so while the underlying node-join problem is unfixed every red run spends about twelve extra minutes (six per suite) before reporting the same thing. Worth stating in the description as an accepted cost, since the run budget is shared with everything queued behind it.
2. cache=unsafe never gets its own measurement here. The description's mechanism — guest fsync serialising the management etcd, operators losing their leases together — is specific and checkable, but this head changes two things at once and its only run failed for a third reason, so nothing in CI isolates the cache change. A run that shows the lease-renewal errors gone would make it evidence instead of reasoning.
3. hack/e2e-prepare-cluster.bats is also touched by #3294 (the per-node data pool grows to 300G there). Whichever lands second will need to reconcile the -drive and qemu-img lines by hand.

The latest push (the "fix" commit) is a pure rebase onto the new base, not a content change: both commits kept their original author dates and only the committer dates moved, and the rationale comment at hack/e2e-chainsaw/_lib/run-kubernetes.sh:447-461 is byte-for-byte unchanged. So the blocking concern raised earlier is still open.

Core issue: the comment justifies widening the node-join budget 12m→18m as host-storage-pressure delaying DataVolume imports. But the failing run on this head shows the worker DataVolumes at Succeeded/100%, both VMIs Running+Ready with margin to spare, and the tenant-CSR capture printing "No resources found" — i.e. the kubelet never submitted a CSR across the full 18m. The limiting term was never the import margin; widening the deadline masks a deterministic failure and the misleading rationale outlives the PR.

Please either land a green run on this head, or re-anchor the justification on the measured terms (CSR issuance + CSR→Ready) and drop the "removes the flakiness" claim. The cache=unsafe change itself is fine.

LGTM — the mechanism the widening writes into the code is reproduced on a main-only run, which retires the blocker I raised earlier, and cache=unsafe is a correct fit for guests that get killed at teardown.

Withdrawing my earlier blocker

I blocked on the rationale comment at hack/e2e-chainsaw/_lib/run-kubernetes.sh:447 for naming a cause the runs contradicted. Both pieces of evidence I built that on were wrong.

The sequence the comment describes is reproduced on main without this PR, in the nightly full-suite job 90471679906 from this morning, where both kubernetes suites went red with fewer than 2 tenant nodes Ready within 12m. In kubernetes-latest the two worker disk importer pods logged FailedScheduling ... 0/3 nodes are available: 3 node(s) had untolerated taint {drbd.linbit.com/lost-quorum: }, the worker VMIs were then created at 05:14:19 and 05:14:20 against a wait that started around 05:12 — the comment's "~2m into this wait" — the guest agent connected only at 05:21:27 and 05:22:31, and the deadline fired at 05:24:06 with zero Nodes. Taint delays the import, the guest then runs out of clock: that is the claim, on a run carrying neither of your changes.

My other point — "no kubelet CSR at all across the full 18 minutes" — read the wrong half of capture (c). On a Talos tenant kubectl get csr is empty by design, because node certificates go through the talos-csr-signer sidecar rather than the CSR API. In that same nightly the list prints No resources found while the signer log records both worker certs signed, at 05:21:41 and 05:22:24. An empty CSR list is not evidence of a missing registration attempt, so it cannot carry the argument I hung on it.

Non-blocking follow-ups

1. 18m buys margin but does not bound the failure. In the same nightly the tenant cilium HelmRelease hit its own 10m install timeout, Flux ran an uninstall remediation, and a second 10m install was still in flight when the deadline fired — InstallFailed ... timeout waiting for: [Deployment/cozy-cilium/cilium-operator status: 'InProgress'] followed by UninstallSucceeded ... uninstall remediation, in both suites and twice for kubernetes-previous. Worst case that cycle is 10m + 10m, so a run can still spend 18m without ever getting a Ready node. Whatever comes after this probably has to touch that install timeout or the remediation policy, not this deadline.
2. On the inline note about the enclosing 40m step: the budget holds, with numbers. The failing kubernetes-latest suite in that nightly took 1429s (~24m) with the 12m deadline, so 18m puts the worst observed run near 30m, and the green run on this head took 1013s (~17m). No change needed.
3. cache=unsafe removes the flush gate, not host writeback pressure. The e2e runner has 128 GB and the three guests claim 72 GiB of it, so once dirty pages for the 50G system and 200G data images reach the kernel's dirty ratio, the guest stalls on writeback throttling instead of on fsync. Worth knowing before the next round of "still flaky" gets attributed elsewhere.
4. The release note says this "removes the cross-operator leader-election flakiness". The phenomenon is real — the same nightly captured management-side cilium-operator lease failures with context deadline exceeded and Failed to renew lease between 03:28 and 03:32, restarts=2 — but nothing in CI isolates the cache change, so that sentence states a hypothesis as a result. Describing what was done rather than what it fixes keeps the changelog honest.
5. #3294 is open and touches both files this PR changes, not only the bats file. Whichever lands second reconciles the -drive lines and the node-join block by hand.

NOT LGTM — correcting the approval I posted a few minutes ago: I approved before checking what 18m actually did on this branch, and it went red six times out of six with the same marker, which contradicts the conclusion the comment writes into the code. Everything else in that review stands, including the withdrawal below.

Still withdrawn: the misattribution charge

My earlier blocker said the comment at hack/e2e-chainsaw/_lib/run-kubernetes.sh:447 named a cause the runs contradicted. That part was wrong and stays withdrawn. The sequence it describes is reproduced on main without this PR, in this morning's nightly full-suite job 90471679906, where both kubernetes suites went red with fewer than 2 tenant nodes Ready within 12m. In kubernetes-latest the two worker disk importer pods logged FailedScheduling ... 0/3 nodes are available: 3 node(s) had untolerated taint {drbd.linbit.com/lost-quorum: }, the worker VMIs were created at 05:14:19 and 05:14:20 against a wait that started around 05:12 — the comment's "~2m into this wait" — the guest agent connected only at 05:21:27 and 05:22:31, and the deadline fired at 05:24:06 with zero Nodes. Taint delays the import, the guest then runs out of clock: that is the claim, on a run carrying neither of your changes.

My other point was worse. "No kubelet CSR at all across the full 18 minutes" read the wrong half of capture (c): on a Talos tenant kubectl get csr is empty by construction, because node certificates go through the talos-csr-signer sidecar rather than the CSR API. In that same nightly the list prints No resources found while the signer log records both worker certs signed, at 05:21:41 and 05:22:24.

Blocker

B1: 18m did not restore margin on this branch — six suites, six times, same marker

File: hack/e2e-chainsaw/_lib/run-kubernetes.sh:447-461

Issue: the comment closes with "this is load-induced slowness, not a stuck bring-up; 18m restores margin". The branch's own CI says otherwise, and the reason it says otherwise is a mechanism the comment does not mention.

Evidence: run 30274169767, head 18c78e12, both halves of this change in effect, three attempts. Every attempt failed both heavy suites, and each carries fewer than 2 tenant nodes Ready within 18m twice — 6 of 6, with suite durations of 1489s/1798s, 1775s/1792s and 1742s/2055s. All three attempts also carry InstallFailed on kubernetes-test-latest-version-cilium and kubernetes-test-previous-version-cilium. That is the loop: the tenant cilium HelmRelease has timeout: 10m and no explicit install.strategy, so Flux's default RemediateOnFailure uninstalls the release between attempts. From the uninstall onward the tenant cluster has no CNI, and a node without CNI cannot reach Ready no matter how much budget is left. This morning's nightly on main shows the same pair of events (InstallFailed ... timeout waiting for: [Deployment/cozy-cilium/cilium-operator status: 'InProgress'], then UninstallSucceeded ... uninstall remediation) in both suites, twice for kubernetes-previous.

Impact: the number is defensible — a 10m install plus a remediation cycle can exceed 18m, so the budget should not be the first thing to fire. The sentence claiming it restores margin is not, and it is the sentence that outlives the PR. The next person to hit this reads "cause identified, margin restored" and widens the budget again instead of looking at the release that keeps taking CNI away.

Fix: keep 18m and rewrite the conclusion around what is measured. The terms are start-of-clock to VMI (2–3m observed), guest boot to first Talos CSR (8m53s–11m46s observed), and CSR to Ready (never observed below ~5m, because no failing run gets there) — the sum is what overruns, not any single term. Then say what the widening does rather than what it fixes: it stops the deadline from being the first thing to fire, so the next failure lands on the cilium install timeout instead. spec.install.strategy.name: RetryOnFailure on that HelmRelease is served by the v2 CRD already in the cluster and would stop remediation from removing CNI; that is the follow-up this comment should point at.

Non-blocking follow-ups

1. On the inline note about the enclosing 40m step: the budget holds, with numbers behind it. The failing kubernetes-latest suite in this morning's nightly took 1429s (~24m) with the 12m deadline, so 18m puts the worst observed run near 30m, and the green attempt on this head took 1013s (~17m). No change needed.
2. cache=unsafe removes the flush gate, not host writeback pressure. The e2e runner has 128 GB and the three guests claim 72 GiB of it, so once dirty pages for the 50G system and 200G data images reach the kernel's dirty ratio the guest stalls on writeback throttling instead of on fsync. Worth knowing before the next round of "still flaky" gets attributed elsewhere.
3. The release note says this "removes the cross-operator leader-election flakiness". The phenomenon is real — the same nightly captured management-side cilium-operator lease failures with context deadline exceeded and Failed to renew lease between 03:28 and 03:32, restarts=2 — but no run isolates the cache change, and the one green attempt on this head sits next to a base that also goes green without these changes. Describing what was done rather than what it fixes keeps the changelog honest.
4. #3294 is open and touches both files this PR changes, not only the bats file. Whichever lands second reconciles the -drive lines and the node-join block by hand.

Not a second block — Aleksei Sviridkin (@lexfrei)'s CHANGES_REQUESTED already stands and I am not stacking another on top of it. One substantive ask, plus a scope correction that changes the CI picture enough that it should be on the record before anyone acts on the standing review.

The CI picture moved: the current head is green

The diff content is byte-identical to 18c78e12 (both files hash to the same blobs, author dates unchanged), so the push was a pure rebase — but the base moved 66c96b432 → d598e672f, and on the current head the heavy suites are 4/4 green. Run 30375214774: attempt 1 (job 90333030340) kubernetes-latest PASS 1069.22s and kubernetes-previous PASS 1261.55s with only chainsaw/harbor failing; attempt 2 (job 90376754751) all 43 suites PASS, 1012.86s and 972.07s. The standing 6 of 6 red at 18m is run 30274169767 on the superseded base. Same diff, opposite outcome — which points at the base, not at either half of this PR.

cache=unsafe: land it as-is

I checked the three things that could make it unsafe and all are absent: nothing on the host ever reads system.img / data.img / seed.img (no consumer anywhere in hack/ or .github/workflows/ outside hack/e2e-prepare-cluster.bats), teardown is a plain kill $(cat srv*/qemu.pid) at line 24 after which nothing reads them, and nothing in the e2e reboots, resets or restarts these guests — so no filesystem is ever re-read after flushes were dropped. Losing unflushed data on a host crash costs a run that was being discarded anyway. Worth noting the delta is narrower than the title reads: QEMU's default here is already cache=writeback, so this only adds cache.no-flush=on, which removes the flush gate but not host writeback throttling — three guests claim 72 GiB (-m 24576 × 3) of a 128 GB runner while the 50G and 200G images are written, so once dirty pages hit the kernel dirty ratio the guest stalls on writeback instead of on fsync. Partial, not ineffective.

The one ask: the rationale comment, not the number

hack/e2e-chainsaw/_lib/run-kubernetes.sh:447-461 attributes the overrun to a drbd.linbit.com/lost-quorum taint delaying the worker DataVolume imports and concludes "this is load-induced slowness, not a stuck bring-up; 18m restores margin". In its cited run 30260770694 (job 89962336210) the taint is real, but in the 12:18:06 event dump the importer pods' FailedScheduling and Scheduled events sit in the same 15m age bucket — under a minute apart — the container Started a minute later, and the imports finished (Succeeded / 100.0% / Import Complete). The ~2m into this wait VMI figure is accurate (12:04:45 and 12:05:08 against a wait starting 12:02:58) but structural, as the comment itself says two sentences earlier when it notes the machinedeployment .status.replicas=2 gate clears while the VMs are still only Machine objects. So the chain taint → delayed imports → margin gone is measurably too small to carry the conclusion.

docs/agents/e2e-testing.md:123 does permit a longer wait when justified in-line, so the widening is not forbidden by convention — but line 112 says to fix the cause rather than widen the timeout, and an in-line justification that names the wrong cause is worse than none, because the sentence outlives the PR and sends the next reader to widen the budget again. Rewriting it around the measured terms — wait-start to VMI ≈2m (structural), VMI to Node-Ready ≲4m on healthy runs, and a failing run that signs no cert at all — and saying what the number does (stops the deadline being the first thing to fire) rather than that it restores margin or removes flakiness, is enough for me.

What the failing runs actually show, for whoever picks up the real cause

Across all seven failing heavy suites I examined (branch attempts 1/2/3, the nightly 90471679906 on main, and the reference run), the worker DataVolumes all reached Succeeded / 100.0% / Import Complete and kubectl describe nodes on the tenant printed zero node output — no Roles:, no Taints:, no node Name: lines, no KubeletNotReady — while kubectl get csr answered and crust-gather completed, so the API was reachable and there were genuinely zero Node objects at every deadline.

The distribution is bimodal. Healthy: node-Ready ≲5m40s into the wait (green attempt 2 kubernetes-latest, bounded via the post-wait coredns-custom 0 8m1s print inside a 900s script — that ConfigMap is resource-policy: keep and its HelmRelease dependsOn cilium, so it cannot predate node-Ready), with node AGE of 60s/57s at the moment the wait exited. Failing: reference run VMIs Running 9m37s/10m at the deadline with zero certs; branch attempt 3 at 15m20s of guest runtime with nothing in the talos-csr-signer log but its two startup lines. ≲4m versus >9m40s-with-no-progress is a wedge, not slowness, and no budget in this range crosses it — which is why I do not think the number is what matters here in either direction.

Where the data does not distinguish two readings: two branch suites did sign certs late (attempt 1 kubernetes-previous at +14m37s/+15m18s, attempt 2 kubernetes-latest at +16m18s), so something was still progressing past 12m there, and the nightly on main signed at +8m35s and +9m35s/+10m18s against a 12m deadline and still had zero Nodes 1m42s–2m25s later. That is equally consistent with "needed a few more minutes" and with "stalled after cert issuance", and one green run cannot settle the tail.

On the cilium remediation follow-up

The cycle is real and I verified it: interval: 5m, timeout: 10m, install.remediation.retries: -1, no strategy (packages/apps/kubernetes/templates/helmreleases/cilium.yaml), and job 90115738417 carries InstallFailed at −14m and −3m14s with UninstallSucceeded … Helm uninstall remediation at −3m8s. But the causal direction looks inverted: with zero Node objects there is no node whose Ready is blocked by a missing CNI, and cilium cannot become Ready with nothing to schedule its operator and agent onto — so the install cycling reads as a consequence of zero nodes, not their cause.

The field itself is available, for the record: helm-controller/api v1.5.1 (this repo's go.mod) defines InstallStrategy and ActionStrategyRetryOnFailure, and cozystack's own PackageReconciler.buildHelmReleaseSpec already sets Install.Strategy.Name = RetryOnFailure on the HelmReleases it creates — so spec.install.strategy is not absent from the surface, though it is worth confirming against the CRD the tenant Flux actually deploys (2.8.x, packages/system/fluxcd/values.yaml:8) before relying on it. Also worth noting #3229, though adjacent, has a different signature — a node that did register and sits Ready=False KubeletNotReady / cni plugin not initialized.

The open question worth its own issue: why does a booted, Running, import-complete guest sometimes never register a Node for 9m40s–15m20s when a healthy one does it in under 4m?

Non-blocking

1. The release note states an effect as a result ("removing the cross-operator leader-election flakiness"). Nothing isolates it — the lease signal is not comparable across these runs because each failure mode dumps a different diagnostic set (nightly 90471679906: 20 Failed to renew lease; attempt 1 with the change, 90009900608: still 2 plus 2 lost-leader; green runs 0, but they dump almost nothing). Describing what was done rather than what it fixes keeps the changelog honest.
2. If 18m is kept: every red run now spends ~6 extra minutes per heavy suite, and the worst failing suite already ran ~2055s (34m15s) against the 40m step timeout at hack/e2e-chainsaw/kubernetes-latest/chainsaw-test.yaml:22. The headroom holds but is thinner than "well inside" suggests.
3. #3294 is open and touches both files here, not only the bats file. Whichever lands second reconciles the -drive lines and the node-join block by hand — I will take that on my side if this one goes first.