ci(e2e): drop 3x retry on Run E2E + Install Cozystack, wait out gateway.bats tenant teardown - #2558
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughRefactors the PR E2E GitHub Actions job to simplify retries and diagnostics, and increases timeouts/polling across many e2e test scripts to reduce flakiness and tolerate slower provisioning. ChangesE2E Workflow Error Handling
E2E tests and helper scripts — increased wait/delete/poll timeouts
Sequence Diagram(s)sequenceDiagram
participant ComponentA
participant ComponentB
ComponentA->>ComponentB: observable interaction
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Note Gemini is unable to generate a summary for this pull request due to the file types involved not being currently supported. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with 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.
Inline comments:
In @.github/workflows/pull-requests.yaml:
- Line 235: The attempt counter is off because it's initialized to 0 but only
incremented on failures, so first-try success prints "0 attempts" and retry
counts are off; fix by initializing attempt to 1 (so the first run counts as
attempt 1) and keep the existing increment-in-failure logic, ensuring the echo
"Prepare environment completed after $attempt attempts" reflects the actual
number of runs; update the variable initialization for attempt (and verify any
incrementing logic around the retry/failure branch that currently updates
attempt).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 564453a0-d0bf-46d0-bd7e-4d152395185c
📒 Files selected for processing (1)
.github/workflows/pull-requests.yaml
f61250f to
60bddbb
Compare
a13b05b to
2895d94
Compare
Flux helm-controller v1.5.0 (shipped in Flux v2.8) replaced the legacy Helm readiness logic with kstatus-based health checking, which polls Deployment/StatefulSet/Service status and waits for kstatus to assess rollout-ready. This typically adds 10-30s of latency between resource apply and HR condition Ready=True compared to v2.7. Our per-app bats tests had `kubectl wait hr/... --for=condition=ready --timeout=` budgets of 20s, 30s, 60s, 100s — sized for v2.7's faster readiness flip and previously masked by the 3x retry on Run E2E tests (dropped in #2558). Under v2.8 with no retry, every app that runs after the first few hit its HR wait timeout exactly, surfacing as 13 of 16 test failures on PR #2619. Standardise on 5m for HR Ready across all app tests. Tight enough to catch genuine bring-up failures within the per-app job budget, loose enough to absorb kstatus dispatch jitter on a management cluster that is concurrently reconciling multiple tenant-test HRs. Post-HR-Ready checks for app-specific resources (StatefulSet/Service/ PVC/operator CRs) remain — for CR-based apps kstatus only observes that the chart applied, not that the operator finished. Audit of which of those are actually redundant under v2.8 kstatus is queued as a follow-up. Also bumps the 10s "kamajicontrolplane appears" wait in run-kubernetes.sh to 2m, and the kubernetes-addon HR Ready loop from 1m to 5m for the same reason. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
Flux helm-controller v1.5.0 (shipped in Flux v2.8) replaced the legacy Helm readiness logic with kstatus-based health checking, which polls Deployment/StatefulSet/Service status and waits for kstatus to assess rollout-ready. This typically adds 10-30s of latency between resource apply and HR condition Ready=True compared to v2.7. Our per-app bats tests had `kubectl wait hr/... --for=condition=ready --timeout=` budgets of 20s, 30s, 60s, 100s — sized for v2.7's faster readiness flip and previously masked by the 3x retry on Run E2E tests (dropped in #2558). Under v2.8 with no retry, every app that runs after the first few hit its HR wait timeout exactly, surfacing as 13 of 16 test failures on PR #2619. Standardise on 5m for HR Ready across all app tests. Tight enough to catch genuine bring-up failures within the per-app job budget, loose enough to absorb kstatus dispatch jitter on a management cluster that is concurrently reconciling multiple tenant-test HRs. Post-HR-Ready checks for app-specific resources (StatefulSet/Service/ PVC/operator CRs) remain — for CR-based apps kstatus only observes that the chart applied, not that the operator finished. Audit of which of those are actually redundant under v2.8 kstatus is queued as a follow-up. Also bumps the 10s "kamajicontrolplane appears" wait in run-kubernetes.sh to 2m, and the kubernetes-addon HR Ready loop from 1m to 5m for the same reason. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
Flux helm-controller v1.5.0 (shipped in Flux v2.8) replaced the legacy Helm readiness logic with kstatus-based health checking, which polls Deployment/StatefulSet/Service status and waits for kstatus to assess rollout-ready. This typically adds 10-30s of latency between resource apply and HR condition Ready=True compared to v2.7. Our per-app bats tests had `kubectl wait hr/... --for=condition=ready --timeout=` budgets of 20s, 30s, 60s, 100s — sized for v2.7's faster readiness flip and previously masked by the 3x retry on Run E2E tests (dropped in #2558). Under v2.8 with no retry, every app that runs after the first few hit its HR wait timeout exactly, surfacing as 13 of 16 test failures on PR #2619. Standardise on 5m for HR Ready across all app tests. Tight enough to catch genuine bring-up failures within the per-app job budget, loose enough to absorb kstatus dispatch jitter on a management cluster that is concurrently reconciling multiple tenant-test HRs. Post-HR-Ready checks for app-specific resources (StatefulSet/Service/ PVC/operator CRs) remain — for CR-based apps kstatus only observes that the chart applied, not that the operator finished. Audit of which of those are actually redundant under v2.8 kstatus is queued as a follow-up. Also bumps the 10s "kamajicontrolplane appears" wait in run-kubernetes.sh to 2m, and the kubernetes-addon HR Ready loop from 1m to 5m for the same reason. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> (cherry picked from commit 5ef02ee)
2895d94 to
f3601ba
Compare
c5e28d0 to
1bc7137
Compare
…(folds #2612) (#2602) ## What this PR does Upgrades Flux v2.7.3 → v2.8.0 across both the vendored tenant chart and the embedded management-cluster manifests, and folds in the chart fixes that become hard errors under the new strict helm-controller v1.5. Flux v2.8's helm-controller v1.5.0 ships: - **Server-Side Apply with `--force-conflicts`** — strict CRD schema enforcement; misplaced fields (silently dropped on v2.7) now fail the apply. - **kstatus-based health checking** by default — parent HR waits for every applied resource (including child HRs) to be Ready before flipping its own Ready, surfacing latent ordering deadlocks. - **Helm v4 embedded** — `upgrade.force: true` is deprecated and now conflicts with SSA. - **`HelmRelease.spec.healthCheckExprs`** — prerequisite for proper readiness gating (used in PR #2601 split). Folds in PR #2612 (kubevirt-instancetypes null TPM fix) since the same Flux upgrade triggers it. ## Commits **Flux upgrade itself:** - `feat(fluxcd)`: bump `flux-operator` / `flux-instance` vendored charts to v0.48.0; web UI opt-in. - `feat(flux)`: regenerate embedded management-cluster manifests via `make update` in `packages/core/flux-aio` (timoni bundle build). **Chart fixes for strict SSA — fields the chart sent that v2.7 silently dropped, v2.8 rejects:** - `fix(kubevirt-instancetypes)`: drop persistent strip that produced null `preferredTPM` (folds #2612). - `fix(foundationdb)`: relocate `faultDomain`, `imageType`, `labels`, `minimumUptimeSecondsForBounce` from inside `automationOptions` to direct children of `spec`. - `fix(kafka)`: place `enableServiceLinks` under `template.pod`, not a phantom `template.spec`. - `fix(vm-instance)`: emit `disk: {}` (not `disk:`/null) when no bus is set. - `fix(platform)`: drop deprecated `upgrade.force: true` from HelmReleases; fix `kafka` WorkloadMonitor `replicas` paths. **Ordering / deadlock fixes under v2.8 kstatus:** - `fix(vpa)`: break circular wait between parent install and nested `vpa-for-vpa` HR. - `fix(kubernetes)`: drop lookup-guarded parent-HR `dependsOn` on tenant addon child HRs (parent waits on child via kstatus, child waited on parent — deadlock). **E2E waits for v2.8 kstatus timing:** - `test(e2e)`: bump app HR-Ready waits to 5m (was 20s–100s under v2.7's faster dispatch). - `test(e2e)`: wait for parent HR Ready before downstream asserts in `run-kubernetes.sh` and `vminstance.bats`. ## Scope discipline This PR is part of the split of #2619 (the consolidated CI fixes branch) into review-friendly pieces. Companion PRs: - **PR #2601** (seaweedfs split) — folded into this PR (commits `29c6afc8`, `0e8b46d7`, `7157158c`, `dccdeb52`, `f880b324`): the seaweedfs-system → seaweedfs-db + seaweedfs-system split, its adoption migration 43 (targetVersion 44), and the configurable db resources all land here, because the strict-SSA `upgrade.force` removal and the kstatus parent-HR timeout bump only make sense together with the split. #2601 is superseded. - **PR #2558** (drop 3× retry on `Run E2E` + `Install Cozystack`) — independent, lands separately. - Several smaller standalone fixes lifted out of #2619 (startup probes, cert-manager `dependsOn`, prepull machinery, CSI HR timeout, NFS/OIDC test improvements) — opened as separate PRs. ## Verification - `helm template` renders cleanly for both `fluxcd` and `fluxcd-operator` packages with `web.enabled=false` (default) and `web.enabled=true`. - Embedded `cmd/cozystack-operator` binary contains the v1.5.0 / v1.8.0 / v2.1.0 controller image strings. - No references to the v0.39-removed `--disable-wait-interruption` flag anywhere in `packages/` or `internal/`. ### Release note ```release-note Flux upgraded to v2.8.0 (helm-controller v1.5 — Helm v4 Server-Side Apply with --force-conflicts, kstatus health checking). When upgrading existing clusters: - Kubernetes 1.33+ is now required for the platform (management) cluster, and for any tenant cluster that enables the optional (default-off) Flux addon — that addon ships the bumped Flux too. - HelmReleases no longer set `upgrade.force: true`. Helm v4 SSA resolves field-ownership conflicts automatically (`--force-conflicts`), but that is not the old client-side replace: immutable-field changes (e.g. StatefulSet volumeClaimTemplates/serviceName) no longer self-heal and require manual recreation — delete the object (e.g. `kubectl delete sts <name> --cascade=orphan`) and let Flux recreate it. - KubeVirt: persistent TPM/EFI is re-enabled for the Windows 11/2k22/2k25 preferences (KubeVirt 1.8 VMPersistentState); each affected VM provisions an extra RWO backend-storage PVC from the default StorageClass. - KubeVirt: the EOL centos.7*/centos.stream8* preferences are retained as deprecated, hidden aliases (`tags: hidden`, `instancetype.kubevirt.io/deprecated: "true"`) — existing VMInstances on these profiles keep rendering and need no action on upgrade, but the profiles are no longer offered for new VMs; repoint to centos.stream9/10 when convenient. The gn1.* GPU instancetypes are likewise retained. - FoundationDB: imageType now reaches the operator (silently dropped pre-SSA); it is pinned to `split` to match the value existing clusters effectively ran, so upgrades stay non-disruptive. Set `imageType: unified` to migrate deliberately. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Flux Status web UI: optional web server, config secret, service port, HTTPRoute/Ingress, network policy, and RBAC roles. * New SeaweedFS DB Helm chart and optional managed DB release. * **Improvements** * CRD/schema enhancements: new provider kinds, validations, variant option, and external checksum refs. * Raised Kubernetes prerequisite to 1.30+; extended e2e timeouts for reliability. * **Chores** * Bumped Flux Operator and Flux versions; documentation links updated to fluxoperator.dev. * **Bug Fixes** * Removed aggressive HelmRelease force-upgrade/install flags. <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/cozystack/cozystack/pull/2602?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) <!-- review_stack_entry_end --> <!-- end of auto-generated comment: release notes by coderabbit.ai -->
1bc7137 to
bb69232
Compare
Audit of 30 successful PR runs found that across 5 sampled failure attempts, **25/25 retries** on `Run E2E tests` failed — the retry loop never recovered a flake, only stretched deterministic failures and tripled diagnostic wall-time. Same data shape on `Install Cozystack`. Drop both. `Prepare environment` keeps its 3x retry — that step is pure infrastructure (Talos image download, sandbox VM boot, network) where transient runner hiccups warrant a retry. On failure, the test step now captures `kubectl get hr -A -o wide` and `kubectl get events -A` under a collapsible group so triage starts with the actual broken-state snapshot. Depends on: - #2508 — installer namespace bootstrap (Helm namespace-ownership conflict) - #2509 — operator HelmRelease config knobs (`seaweedfs-system` 2-min wait race) - #2528 — harbor bucket-secret + BucketInfo gating (harbor ValuesError) - #2529 — objectstorage-controller BucketAccess conflict retry - the daniil/split-vminstance PR (vminstance disk race + VM IP/ready timeouts) - the daniil/split-event-driven PR (existence backstops surfacing real errors) Until those land, dropping the retry will fail CI for unrelated PRs that hit the seaweedfs / harbor / installer / vminstance races. Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> Assisted-By: Claude <noreply@anthropic.com>
Address review feedback from coderabbitai on .github/workflows/pull-requests.yaml:235: $attempt is incremented only on failure, so first-try success printed "completed after 0 attempts". Use $((attempt + 1)) so the message reflects the total number of runs. Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
Mirror the diagnostics group already on `Run E2E tests` to the `Install Cozystack into sandbox` step. Without this, install-step failures (e.g. an HR not reaching Ready inside the bats `kubectl wait` window) surface only as "error: timed out waiting for the condition on helmreleases/<name>" with no cluster snapshot — re-running is the only way to triage, which is the exact pattern the parent commit removed. On failure, dump three groups: - `kubectl get hr -A -o wide` (last 100 lines): which HRs exist and their Ready/Status columns. - `kubectl describe hr` for each not-Ready HR (tail -50): the Conditions block surfaces helm install/upgrade errors, missing CRDs, and chart-render failures — the actual cause. - `kubectl get events -A --sort-by=.lastTimestamp` (last 50): cluster events that often pinpoint the failing pod/PVC/job. Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
bb69232 to
d8d4c71
Compare
The last three gateway tests create nested tenants (gwprop, gwparent/gwchild, rparent/rchild) and deleted them fire-and-forget, with parent and child deletes back-to-back. The file exits in ~25s while the uninstalls run on for minutes: each tenant uninstall blocks on a cleanup Job in cozy-system, and a parent deleted alongside its child wedges on the child's still-terminating namespace. On the tenants helm-controller shard (--concurrent=5) the leftovers from one gateway.bats run — three stuck parent uninstalls, one stuck child uninstall, and the rchild HR still mid-install — occupied exactly 5 workers, so the next test in line (harbor) sat unreconciled for its whole 5m HR-Ready budget and failed without ever erroring. Sequence the teardown instead: delete child, wait for its HR to go away, then delete parent and wait again. The waits are hard failures on purpose — if a tenant uninstall genuinely wedges, gateway.bats is the test that should go red, not whichever app test runs next. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
…eport Run 27057500380 failed on harbor's BucketClaim never reaching bucketReady=true, and neither artifact could explain why: the per-app diagnostics dump has no bucket objects, and cozyreport collects neither COSI resources nor objectstorage-controller / seaweedfs provisioner logs — by the time the report is taken, the bucket state has moved on. Add a COSI module to cozyreport.sh (bucketclaims/bucketaccesses/ buckets/classes as custom-column tables plus full YAML, controller and per-namespace provisioner logs) and a bucket-readiness snapshot to the per-app failure dump in the PR workflow. The COSI CRDs ship no printer columns, so plain kubectl get would show only NAME/AGE — readiness fields are pulled explicitly. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
…licies type-check cozystack-basics ships cluster-wide ValidatingAdmissionPolicies matching tenants.apps.cozystack.io and the Gateway API types. KCM's VAP status type-checker panics on a nil schema when it type-checks a policy against a matched type that isn't resolvable yet; during a cold install this crashes KCM, which stalls cert/token issuance, so cozystack-api never starts and the tenants schema never resolves — a bootstrap deadlock. Add dependsOn edges so the package installs only after cozystack-engine (cozystack-api) and gateway-api-crds are Ready. Drift detection is off on operator-generated HelmReleases, so a template capability gate would drop the policy at first install and never re-add it; ordering is the reliable fix. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> (cherry picked from commit 03f1e7a)
Aleksei Sviridkin (lexfrei)
left a comment
There was a problem hiding this comment.
LGTM — the platform ordering fix is correct, introduces no dependency cycle, is covered by a helm-unittest regression guard, and the full E2E suite is green. Comments below are non-blocking.
What I verified
cozystack-basicsdependsOn ordering (the platform fix):cozystack-basicsships cluster-wide ValidatingAdmissionPolicies matchingtenants.apps.cozystack.io(served bycozystack-apiincozystack-engine) and the Gateway API types (gateways/httproutes/tlsroutesfromgateway-api-crds). Ordering the package after those APIs is the right fix. Verified there is no cycle: nothingdependsOncozystack-basics(it is a dependency-graph leaf),cozystack-enginedepends only onnetworking/cert-manager, andgateway-api-crdshas an emptydependsOn. Rejecting a template capability-gate is justified — drift detection is off on operator-generated HelmReleases, so a gate would render the policy out on first install and never restore it. The full platform helm-unittest suite passes locally (27/27).- Retry removal + diagnostics: the shell logic is sound; the attempt-counter off-by-one is already fixed (
$((attempt + 1))), and$4is the correct READY column inkubectl get hr -A. - gateway.bats teardown: the test runner executes each test under
set -e, so await --for=deletetimeout on a genuinely wedged uninstall fails gateway.bats itself — which is the stated goal. The HR names are correct and the green E2E run confirms the happy path. - cozyreport COSI capture: resource scoping is correct (cluster-scoped
buckets/bucketclasses/bucketaccessclasseswithout-A, namespacedbucketclaims/bucketaccesseswith-A), all best-effort.
Non-blocking follow-ups
- The five new
wait --for=deletecalls inhack/e2e-apps/gateway.bats(lines 493, 551, 553, 681, 683) are unguarded, whereas every other--for=deleteunderhack/e2e-apps/is guarded —etcd.batswith--ignore-not-found, andharbor.bats/kafka.bats/run-kubernetes.shwith2>/dev/null || true. A barekubectl wait --for=deleteon a resource that is already absent at invocation returnserror: no matching resources found(exit 1), which fails the test underset -e. The happy path is safe (the tenant is created earlier in the same test, so the HR exists), but adding--ignore-not-found— asetcd.batsalready does — removes the residual race without weakening the intended "timeout fails the test" behavior. - This PR also carries a platform install-ordering change (
cozystack-basics) that affects every cozystack install, not only CI, yet it is labelledarea/ci/area/testingonly and the release note isNONE. Consider addingarea/platformfor discoverability, and double-checking whether a fresh-install KCM-deadlock fix warrants a release note (keepingNONEis fine if the broken ordering never shipped in a release). - Minor: in the install-failure diagnostics, the
docker exec … | while readpipeline is unguarded (only the innerdescribehas|| true). Underset -eo pipefaila transientdocker execfailure there would skip the trailing "recent events" group before the explicitexit 1. The step still fails correctly; only diagnostic completeness is at risk.
| # uninstall carries over into the next .bats file, starving the | ||
| # helm-controller worker pool (--concurrent=5 on the tenants shard). | ||
| kubectl -n tenant-test delete tenant gwprop --ignore-not-found | ||
| kubectl -n tenant-test wait hr tenant-gwprop --for=delete --timeout=300s |
There was a problem hiding this comment.
Every other --for=delete under hack/e2e-apps/ is guarded against the not-found-at-invocation case (etcd.bats uses --ignore-not-found, harbor.bats/kafka.bats/run-kubernetes.sh use 2>/dev/null || true). A bare kubectl wait --for=delete on a resource that is already absent when the command starts returns error: no matching resources found (exit 1), which fails the test under set -e. Suggest matching etcd.bats and appending --ignore-not-found here (and on the other four new waits): it keeps the intended "timeout on a wedged uninstall fails the test" behavior while removing the residual race. Non-blocking.
A churn-heavy platform install intermittently wedges a pod in
ContainerCreating because the cilium-agent rejects its sandbox with
'[PUT /endpoint/{id}][400] putEndpointIdInvalid "IP ipv4:<X> is already
in use"', repeated until the agent restarts or the install times out.
The disputed IP has no live owner: IPAM and the ipcache released it, but
the agent's in-memory endpointManager still holds a stale Endpoint from a
previously-deleted pod whose CNI DEL was skipped or raced (so
removeReferencesLocked never ran). Neither the operator's CiliumEndpoint
GC nor the agent's veth-based endpoint GC can reap it. This surfaced as
~6 of the post-#2558 e2e failures, each on a different victim pod
(velero node-agent, fluent-bit, fdb-operator, cdi-operator, capi
bootstrap), failing PRs unrelated to the change under test. Tracked
upstream at cilium/cilium#38313 (closed stale); no released or
pre-release Cilium fixes it and no flag disables it.
hack/e2e-cilium-endpoint-leak-healer.sh runs as a file-scoped background
watchdog (setup_file/teardown_file). It detects a wedged pod, confirms
the IP is a true orphan -- an endpoint in the owning node's registry
holds it, that endpoint backs no live Running pod, and a different pod is
wedged requesting the same IP -- then evicts only that endpoint via
'cilium-dbg endpoint disconnect ipv4:<X>' (DELETE /endpoint/{id} ->
unexpose -> removeReferencesLocked). Blast radius is one dead endpoint,
not an agent restart. It is read-only until an orphan is positively
confirmed and refuses (loudly) to touch an endpoint backing a live pod,
so a genuine duplicate-IP bug stays visible rather than masked.
This is an interim CI mitigation, not a product fix; remove the script
and the bats hooks once a fixed Cilium ships.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
Signed-off-by: Ivan Okhotnikov <ivan.okhotnikov@aenix.io>
A churn-heavy platform install intermittently wedges a pod in
ContainerCreating because the cilium-agent rejects its sandbox with
'[PUT /endpoint/{id}][400] putEndpointIdInvalid "IP ipv4:<X> is already
in use"', repeated until the agent restarts or the install times out.
The disputed IP has no live owner: IPAM and the ipcache released it, but
the agent's in-memory endpointManager still holds a stale Endpoint from a
previously-deleted pod whose CNI DEL was skipped or raced (so
removeReferencesLocked never ran). Neither the operator's CiliumEndpoint
GC nor the agent's veth-based endpoint GC can reap it. This surfaced as
~6 of the post-#2558 e2e failures, each on a different victim pod
(velero node-agent, fluent-bit, fdb-operator, cdi-operator, capi
bootstrap), failing PRs unrelated to the change under test. Tracked
upstream at cilium/cilium#38313 (closed stale); no released or
pre-release Cilium fixes it and no flag disables it.
hack/e2e-cilium-endpoint-leak-healer.sh runs as a file-scoped background
watchdog (setup_file/teardown_file). It detects a wedged pod, confirms
the IP is a true orphan -- an endpoint in the owning node's registry
holds it, that endpoint backs no live Running pod, and a different pod is
wedged requesting the same IP -- then evicts only that endpoint via
'cilium-dbg endpoint disconnect ipv4:<X>' (DELETE /endpoint/{id} ->
unexpose -> removeReferencesLocked). Blast radius is one dead endpoint,
not an agent restart. It is read-only until an orphan is positively
confirmed and refuses (loudly) to touch an endpoint backing a live pod,
so a genuine duplicate-IP bug stays visible rather than masked.
This is an interim CI mitigation, not a product fix; remove the script
and the bats hooks once a fixed Cilium ships.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
What this PR does
Drops the 3× retry loop on
Run E2E testsandInstall Cozystack into sandbox.Prepare environmentkeeps its 3× retry — that step is pure infrastructure (Talos image download, sandbox VM boot, network) where transient runner hiccups warrant a retry.On failure, the test step now captures
kubectl get hr -A -o wideandkubectl get events -Aunder a collapsible group so triage starts with the actual broken-state snapshot.Note
An earlier revision of this PR also doubled every bats timeout. That commit was dropped in a rebase and is intentionally not restored: the timeout class that actually matters (per-app HR-Ready waits) has since been standardized at 5m on
main(7b9f286), making a blanket 2× redundant.Fixes gateway.bats teardown leakage. The nested-tenant tests deleted tenants fire-and-forget, parent and child back-to-back. The leftover uninstalls (each blocked on a cleanup Job, parents wedged on still-terminating child namespaces) plus one mid-install child HR occupied exactly 5 workers on the
--concurrent=5tenants helm-controller shard, starving whichever app test ran next — observed as the harbor HR sitting unreconciled for its whole 5m HR-Ready budget in run 27020081550, surfaced by this PR's own retry removal + diagnostics dump. Teardown now deletes child→parent with hardwait hr --for=deletebetween, so a wedged tenant uninstall fails gateway.bats itself, not an innocent neighbor.Why
Audit of 30 successful PR runs found that across 5 sampled failure attempts, 25/25 retries on
Run E2E testsfailed — the retry loop never recovered a flake, only stretched deterministic failures and tripled diagnostic wall-time. Same data shape onInstall Cozystack.Beyond wasted CI time, the retry was hiding ~10 deterministic bugs (Helm namespace-ownership conflict, seaweedfs HR timeout, harbor BucketInfo wiring, vminstance disk race, etc.). Each failure looked like a "flake" because the retry sometimes coincided with whatever transient state had cleared — the retry never fixed the bug, just delayed surfacing.
Dependencies
The deterministic bugs the retry was masking are now fixed on
main:seaweedfs-system2-min wait race within Flux's 5-min reconcile windows) — mergedValuesErroron first install) — mergedCompanion PRs in the #2619 split (independent of this PR, ordering-wise):
This PR does NOT depend on #2602/#2601 — it now touches only the workflow file and gateway.bats teardown, both on top of fresh
main.Surfaced from #2500.
Release note
Summary by CodeRabbit