Uh oh!
There was an error while loading. Please reload this page.
fix(ci): wait for the metrics.k8s.io APIService before the e2e helm install (#863) - #871
Conversation
…nstall (#863) `Seal-check egress-enforcement (k3d)` failed intermittently BEFORE it tested anything: "resourceMonitor is enabled but the metrics.k8s.io/v1beta1 API is not registered." This is a harness race, not a chart defect. k3s registers its bundled metrics-server addon — and the v1beta1.metrics.k8s.io APIService the resource-monitor preflight looks up (client#823) — ASYNCHRONOUSLY, after nodes report Ready. The e2e harnesses gated only on `kubectl wait --for=condition=Ready nodes` and then helm-installed, so on a fast runner the install beat the addon and the preflight `fail`ed the whole release (#862 false-failed at 26s while #861 passed at 51s, neither touching the chart or these scripts). Fix: add e2e_wait_for_metrics_apiservice to scripts/tests/lib/e2e-common.sh and call it after node-Ready, before the helm install, in every harness that installs a preflight-carrying chart directly: e2e-seal-check.sh, e2e-full-seal.sh, and e2e-auto-upgrade.sh (whose first install is the last PUBLISHED chart, which carries the preflight too). The helper POLLS for the APIService to EXIST first — `kubectl wait` errors NotFound on a not-yet-created named object, so a bare `kubectl wait --for=condition=Available` would just swap one red for another in the same window — then best-effort waits for Available. It mirrors the production installer's _wait_for_metrics_apiservice (lib/install-client-helm.sh, client#553), which faces the identical race. Rejected the weaker options (resourceMonitor:false / metricsServerPreflight:false): both go green only by deleting the #823 preflight coverage this seal-check exists to exercise on a real cluster. New e2e-metrics-apiservice-wait.bats pins the invariant (all three harnesses wait before their first install; the wait polls-for-existence before the condition wait) so the guard cannot silently drift back out. Verified on real k3d (rancher/k3s:v1.36.3-k3s1): reproduced the exact resource-monitor-daemonset.yaml:69 fail when the APIService is absent; confirmed the helper blocks until registered+Available and the preflight then renders satisfied-by-apiservice. `make lint` clean; full bats suite failure set identical to develop tip (zero failures added). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
saadqbal
left a comment
There was a problem hiding this comment.
The diagnosis is the valuable half. #862 false-failing at 26s while #861 passed at 51s, neither touching the chart or these scripts, is a timing signature rather than a hunch — and kubectl wait on a not-yet-created named object errors NotFound rather than waiting is the non-obvious fact that separates fixing this from re-reddening it with a different message. The issue's first-draft one-liner would have done exactly that, and you said so.
Polling for existence and then best-effort waiting for Available is right, and mirroring the production installer's _wait_for_metrics_apiservice rather than inventing a second shape is right too. So is refusing resourceMonitor: false / metricsServerPreflight: false — both go green by deleting the client#823 coverage this seal-check exists to exercise, which is going green by removing the thing under test.
Both bats assertions are genuinely derived, which I checked: call_ln < install_ln from real line numbers, get_ln < wait_ln from the awk-extracted function body, and [ -n … ] guards that fail when a grep finds nothing — including "no 'helm install' in $f — test assumption broken", the test checking its own premise. The call-line regex being start-of-line anchored also means a mention in a comment can't satisfy it.
One finding, non-blocking: the assertions are derived but the scope is a hardcoded list.HARNESSES names three scripts. A fourth harness that helm-installs a preflight-carrying chart is simply invisible to the guard — it passes by not being enumerated.
The evidence that this is the part that misses things is in your own PR: e2e-auto-upgrade.sh was "same class, caught in self-review". Your first pass at the list missed one of three, and the guard cannot notice the next one.
Two sibling guards in this repo took the other road within the same week — client#860's CronJob check reads every CronJob out of the rendered manifests and holds no list of names (refusing outright on zero), and backend#2655 derives its terraform test directories from the tree. Deriving here looks tractable: the exclusion reason you give — the e2e-cluster/journey/proxy harnesses stop before the tracebloc install, so they render no resource-monitor and carry no preflight — is itself a property of the script (does it helm install the tracebloc chart at all?) rather than a fact only a human knows. Worth doing when the fourth harness appears, if not now.
Green, no threads, verified on real k3d. 👍
Closes#863.
The flake
Seal-check egress-enforcement (k3d)failed intermittently before it tested anything:This is a harness race, not a chart defect. k3s registers its bundled metrics-server addon — and the
v1beta1.metrics.k8s.ioAPIService the resource-monitor preflightlookups (client#823) — asynchronously, after nodes report Ready. The e2e harnesses gated only onkubectl wait --for=condition=Ready nodesand then helm-installed, so on a fast runner the install beat the addon and the preflightfailed the whole release. Evidence: #862 false-failed at 26s while #861 passed at 51s, neither touching the chart or these scripts.The fix
e2e_wait_for_metrics_apiservice(new, inscripts/tests/lib/e2e-common.sh), called after node-Ready and before the helm install in every harness that installs a preflight-carrying chart directly:e2e-seal-check.sh— the reported flake (local chart).e2e-full-seal.sh— its secret-ful sibling (local chart).e2e-auto-upgrade.sh— same class: its first install is the last published chart, which carries the preflight too (fix-the-class, caught in self-review).The helper polls for the APIService to EXIST first, then best-effort waits for
Available. This matters: the object itself appears late (not merely its condition), andkubectl waiton a not-yet-created named object errorsNotFoundrather than waiting — so a barekubectl wait --for=condition=Available apiservice/...(the issue's first-draft one-liner) would just swap one red for another in the same window. It mirrors the production installer's_wait_for_metrics_apiservice(lib/install-client-helm.sh, client#553), which faces the identical race.Rejected the weaker options (
resourceMonitor: false/nodeAgents.metricsServerPreflight: false): both go green only by deleting the client#823 preflight coverage this seal-check exists to exercise on a real cluster.e2e-metrics-apiservice-wait.bats(new) pins the invariant — all three harnesses wait before their first install, and the wait polls-for-existence before the condition wait — so the guard cannot silently drift back out.Verification
Verified on real k3d (
rancher/k3s:v1.36.3-k3s1):helm install --dry-run=serverfails atclient/templates/resource-monitor-daemonset.yaml:69with the issue's error.tracebloc.io/metrics-server-preflight: satisfied-by-apiservice.set -euo pipefail.make lintclean; fullbatssuite failure set identical to develop tip (zero failures added); new guard test green.Note
Low Risk
Changes only CI e2e bring-up scripts and static bats guards; no production install or chart template behavior is modified.
Overview
Fixes intermittent Seal-check egress-enforcement (k3d) failures where Helm aborted before any seal test with metrics.k8s.io/v1beta1 API is not registered — a harness race because k3s registers the metrics APIService after nodes go Ready, while the chart’s resource-monitor preflight requires it at install time.
Adds
e2e_wait_for_metrics_apiserviceine2e-common.sh: poll untilv1beta1.metrics.k8s.ioexists (avoiding a barekubectl waitNotFound on a late-created object), then a best-effort Available wait. The three harnesses that install preflight-carrying charts call it after node-Ready and before their firsthelm install:e2e-seal-check.sh,e2e-full-seal.sh, ande2e-auto-upgrade.sh(published chart path included).e2e-metrics-apiservice-wait.batslocks in that contract so the wait cannot drift out of any harness or revert to existence-lesskubectl wait.Reviewed by Cursor Bugbot for commit 89f19f6. Bugbot is set up for automated code reviews on this repo. Configure here.