test(e2e): make kuberture split-horizon probes survive Cilium ingress-IP race - #3430
Conversation
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
📝 WalkthroughWalkthroughChangesEDNS probe routing validation
Estimated code review effort: 3 (Moderate) | ~25 minutes Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant Chainsaw
participant ProbeJobs
participant ProbePods
Chainsaw->>ProbeJobs: Assert public and internal Jobs Complete
ProbeJobs->>ProbePods: Create or recreate labeled probe pods
Chainsaw->>ProbePods: Capture succeeded pod logs by Job-name labels
Chainsaw->>Chainsaw: Assert hostname and route isolation
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
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 `@hack/e2e-chainsaw/kuberture/probes.yaml`:
- Line 49: Lower the backoffLimit for both probe Jobs in probes.yaml so their
maximum retry duration stays within the 4-minute Chainsaw assertion window;
preserve the existing job behavior otherwise.
🪄 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: 00049f83-4807-4f03-99de-730961c5eb1a
📒 Files selected for processing (2)
hack/e2e-chainsaw/kuberture/chainsaw-test.yamlhack/e2e-chainsaw/kuberture/probes.yaml
| - --domain-filter=cozystack.test | ||
| - --log-level=info | ||
| - --log-format=text | ||
| backoffLimit: 6 |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
set -euo pipefail
printf '\n== probes.yaml ==\n'
cat -n hack/e2e-chainsaw/kuberture/probes.yaml | sed -n '1,220p'
printf '\n== chainsaw-test.yaml ==\n'
cat -n hack/e2e-chainsaw/kuberture/chainsaw-test.yaml | sed -n '120,190p'
printf '\n== search for backoffLimit / timeout references ==\n'
rg -n "backoffLimit|timeout: 4m|Complete" hack/e2e-chainsaw/kuberture -SRepository: cozystack/cozystack
Length of output: 8897
🌐 Web query:
Kubernetes Job controller backoffLimit exponential backoff 10s 20s 40s 80s 160s 320s official docs
💡 Result:
In Kubernetes, when a Job's Pods fail, the Job controller recreates them using an exponential backoff delay [1]. This delay sequence starts at 10 seconds and doubles with each consecutive failure (10s, 20s, 40s, 80s, 160s, 320s) [1][2]. The backoff delay is capped at 6 minutes (360 seconds) [1][3]. Key details regarding this mechanism include: - Default Backoff: The initial delay is 10 seconds [3]. - Progression: The delay doubles for each subsequent failure (10s, 20s, 40s, 80s, 160s, 320s) until it reaches the cap [2]. - Cap: The maximum backoff duration is 6 minutes (360 seconds) [1][3]. - Implementation: The logic is handled by the Job controller to determine the delay before creating a replacement Pod [2][4]. If the number of Pod failures reaches the.spec.backoffLimit, the Job is marked as failed [1][5]. Note that this exponential backoff applies to Pod failures counted towards the.spec.backoffLimit (or.spec.backoffLimitPerIndex for indexed jobs) [1][6]. Certain failures, such as those ignored via a Pod failure policy, do not increment the counter and therefore do not trigger this backoff sequence [7][5][8].
Citations:
- 1: https://kubernetes.io/docs/concepts/workloads/controllers/job/
- 2: https://github.com/kubernetes/kubernetes/blob/e9b95827/pkg/controller/job/backoff_utils.go
- 3: https://github.com/kubernetes/kubernetes/blob/752c4b7f0b3630fb58f8489cb30748be79cde4f2/pkg/controller/job/job_controller.go
- 4: Decouple batch/job back-off logic from workqueues kubernetes/kubernetes#114768
- 5: https://kubernetes.io/docs/reference/kubernetes-api/batch/job-v1/
- 6: https://kubernetes.io/blog/2025/05/13/kubernetes-v1-33-jobs-backoff-limit-per-index-goes-ga/
- 7: https://kubernetes.io/docs/tasks/job/pod-failure-policy/
- 8: https://kubernetes.io/blog/2024/08/19/kubernetes-1-31-pod-failure-policy-for-jobs-goes-ga/
🏁 Script executed:
python3 - <<'PY'
import math
# Kubernetes Job exponential backoff delays (seconds) for successive pod retries:
delays = [min(10 * (2 ** i), 360) for i in range(6)]
print("retry_delays_s =", delays)
print("total_backoff_s =", sum(delays))
print("total_backoff_min =", round(sum(delays) / 60, 2))
PYRepository: cozystack/cozystack
Length of output: 248
backoffLimit: 6 can outlast the 4m Chainsaw asserts. Both probe Jobs can sit in controller backoff for ~10.5m before failing, so repeated IPAM flakes may still time out the Complete checks. Lower the backoff limit or raise the timeout.
🤖 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-chainsaw/kuberture/probes.yaml` at line 49, Lower the backoffLimit
for both probe Jobs in probes.yaml so their maximum retry duration stays within
the 4-minute Chainsaw assertion window; preserve the existing job behavior
otherwise.
…-IP race The split-horizon-routing-proof step ran two external-dns probes as bare Pods and polled them by fixed name. When the Cilium host-scope IPAM + Gateway API restore race (cilium/cilium#46799) hands a probe the per-node reserved:ingress IP, the sandbox is rejected with "IP already in use" and the e2e leak-healer deletes the wedged pod, expecting a controller to reschedule it. A bare Pod has no controller, so the delete was terminal: the probe vanished and the step failed polling a NotFound pod. Run the probes as Jobs so the pod is re-created on a fresh IP after the healer's delete, assert each Job reaches Complete instead of polling a fixed Pod name, and read the logs by job-name label. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la>
ebd0167 to
436a19d
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
hack/e2e-chainsaw/kuberture/probes.yaml (1)
54-59: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winHarden both probe containers.
Neither template prevents privilege escalation or requires a non-root UID. Add restrictive security contexts, after confirming the pinned image’s runtime UID supports
runAsNonRoot.Proposed hardening
spec: + securityContext: + runAsNonRoot: true + seccompProfile: + type: RuntimeDefault serviceAccountName: kuberture-e2e-edns-probe restartPolicy: Never containers: - name: edns + securityContext: + allowPrivilegeEscalation: false + capabilities: + drop: ["ALL"] image: registry.k8s.io/external-dns/external-dns:v0.20.0Also applies to: 84-89
🤖 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-chainsaw/kuberture/probes.yaml` around lines 54 - 59, Harden both probe containers, including the edns container and the other template referenced by the review, with restrictive container security contexts. Confirm the pinned images run successfully as a non-root user, then require non-root execution and disable privilege escalation for each container. Preserve the existing probe behavior and image configuration.Source: Linters/SAST tools
hack/e2e-chainsaw/kuberture/chainsaw-test.yaml (1)
142-163: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winConsider failing fast on
Failed, not just waiting forComplete.If a Job exhausts
backoffLimit(6) it reaches aFailedcondition, notComplete. As written, theassertonCompletewill just spin for the full 4m timeout in that case instead of surfacing the real failure quickly. Chainsaw'serror:operation can short-circuit this by failing the step immediately if theFailedcondition is observed.♻️ Suggested addition (repeat per Job)
- assert: timeout: 4m resource: apiVersion: batch/v1 kind: Job metadata: name: kuberture-e2e-edns-public namespace: cozy-kuberture status: (conditions[?type == 'Complete']): - status: "True" + - error: + resource: + apiVersion: batch/v1 + kind: Job + metadata: + name: kuberture-e2e-edns-public + namespace: cozy-kuberture + status: + (conditions[?type == 'Failed']): + - status: "True"🤖 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-chainsaw/kuberture/chainsaw-test.yaml` around lines 142 - 163, Update the assertions for kuberture-e2e-edns-public and kuberture-e2e-edns-internal to add a Chainsaw error check for each Job’s Failed condition, so the step fails immediately when either Job reaches Failed while retaining the existing Complete assertion.
🤖 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 `@hack/e2e-chainsaw/kuberture/chainsaw-test.yaml`:
- Around line 164-175: Update the log collection in the script to select only
pods with status.phase=Succeeded for each Job before running leak-detection
assertions. Keep the existing Job-name label filters, log options, and separate
pub_logs/int_logs variables, but ensure kubectl logs targets the successful pod
rather than aggregating stale failed attempts.
---
Nitpick comments:
In `@hack/e2e-chainsaw/kuberture/chainsaw-test.yaml`:
- Around line 142-163: Update the assertions for kuberture-e2e-edns-public and
kuberture-e2e-edns-internal to add a Chainsaw error check for each Job’s Failed
condition, so the step fails immediately when either Job reaches Failed while
retaining the existing Complete assertion.
In `@hack/e2e-chainsaw/kuberture/probes.yaml`:
- Around line 54-59: Harden both probe containers, including the edns container
and the other template referenced by the review, with restrictive container
security contexts. Confirm the pinned images run successfully as a non-root
user, then require non-root execution and disable privilege escalation for each
container. Preserve the existing probe behavior and image configuration.
🪄 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 Plus
Run ID: f631455a-6730-4255-9dd5-6ee2af773dc4
📒 Files selected for processing (2)
hack/e2e-chainsaw/kuberture/chainsaw-test.yamlhack/e2e-chainsaw/kuberture/probes.yaml
| - script: | ||
| timeout: 1m | ||
| content: | | ||
| set -eu | ||
| for probe in public internal; do | ||
| deadline=$(( $(date +%s) + 180 )) | ||
| until kubectl -n cozy-kuberture get pod "kuberture-e2e-edns-${probe}" \ | ||
| -o jsonpath='{.status.phase}' 2>/dev/null | grep -Eq '^(Succeeded|Failed)$'; do | ||
| [ "$(date +%s)" -ge "$deadline" ] && break | ||
| sleep 3 | ||
| done | ||
| # Guard the capture: if the loop timed out before the Pod existed, | ||
| # an unguarded read fails under `set -e` and skips the dump below. | ||
| if ! phase=$(kubectl -n cozy-kuberture get pod "kuberture-e2e-edns-${probe}" -o jsonpath='{.status.phase}' 2>/dev/null); then | ||
| phase="Missing" | ||
| fi | ||
| if [ "$phase" != "Succeeded" ]; then | ||
| echo "==== probe ${probe} failed: phase=${phase} ====" | ||
| kubectl -n cozy-kuberture describe pod "kuberture-e2e-edns-${probe}" || true | ||
| kubectl -n cozy-kuberture logs "pod/kuberture-e2e-edns-${probe}" || true | ||
| exit 1 | ||
| fi | ||
| done | ||
|
|
||
| pub_host=kuberture-e2e-public.cozystack.test | ||
| int_host=kuberture-e2e-internal.cozystack.test | ||
| pub_logs=$(kubectl -n cozy-kuberture logs pod/kuberture-e2e-edns-public) | ||
| int_logs=$(kubectl -n cozy-kuberture logs pod/kuberture-e2e-edns-internal) | ||
|
|
||
| # Read each Job's succeeded pod by its job-name label. The healer may | ||
| # have deleted earlier wedged pods, but the Complete condition asserted | ||
| # above guarantees one succeeded pod remains to carry the logs. | ||
| pub_logs=$(kubectl -n cozy-kuberture logs --tail=-1 --max-log-requests=8 -l batch.kubernetes.io/job-name=kuberture-e2e-edns-public) | ||
| int_logs=$(kubectl -n cozy-kuberture logs --tail=-1 --max-log-requests=8 -l batch.kubernetes.io/job-name=kuberture-e2e-edns-internal) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Files matching chainsaw-test.yaml:"
fd -a 'chainsaw-test\.yaml$' . | sed 's#^\./##'
echo
echo "Target file excerpt:"
if [ -f hack/e2e-chainsaw/kuberture/chainsaw-test.yaml ]; then
wc -l hack/e2e-chainsaw/kuberture/chainsaw-test.yaml
sed -n '130,205p' hack/e2e-chainsaw/kuberture/chainsaw-test.yaml | nl -ba -v130
fi
echo
echo "Probes references:"
rg -n "kuberture-e2e-edns-(public|internal)|backoffLimit|restartPolicy: Never|Complete|leak|job-name|cozystack.test" hack -SRepository: cozystack/cozystack
Length of output: 1845
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Inspect adjacent assertions and pod labels in chainsaw-test.yaml:"
sed -n '80,185p' hack/e2e-chainsaw/kuberture/chainsaw-test.yaml | nl -ba -v80
echo
echo "Inspect related probes YAML references:"
fd -a 'probes\.yaml$|.*kuberture.*\.ya?ml$' hack/e2e-chainsaw hack | sed 's#^\./##'
for f in $(fd '.*kuberture.*\.ya?ml$|probes\.yaml$' hack -d 3); do
echo "--- $f"
rg -n "name: kuberture-e2e-edns-(public|internal)|job-name: kuberture-e2e-edns-(public|internal)|backoffLimit|restartPolicy|cozystack\.test|reserved:ingress" "$f" -S || true
doneRepository: cozystack/cozystack
Length of output: 263
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Programmatically inspect the test around log aggregation and probe job specs
python3 - <<'PY'
from pathlib import Path
p = Path('hack/e2e-chainsaw/kuberture/chainsaw-test.yaml')
if not p.exists():
print("missing target file")
raise SystemExit
lines = p.read_text().splitlines()
for i, line in enumerate(lines, 1):
if 'kubectl -n cozy-kuberture logs' in line or 'batch.kubernetes.io/job-name=kuberture-e2e-edns-public' in line or 'batch.kubernetes.io/job-name=kuberture-e2e-edns-internal' in line or 'grep -Fq "$int_host"' in line or 'grep -Fq "$pub_host"' in line:
print(f"{i}: {line}")
for filename, selector in [
('hack/e2e-chainsaw/kuberture/chainsaw-test.yaml', 'job-name=kuberture-e2e-edns-public'),
('hack/e2e-chainsaw/kuberture/chainsaw-test.yaml', 'job-name=kuberture-e2e-edns-internal'),
]:
text = Path(filename).read_text()
print(f"\n--- {filename}: selector {selector} context")
idx = text.find(selector)
if idx != -1:
start = max(0, text.rfind('\n', 0, idx-200))
end = text.find('\n', idx+500)
chunk = text[start:end if end!=-1 else None]
print(chunk[:800])
# Also find exact job name in nearby job selector, if YAML-like text is present
for path in Path('hack').rglob('*.yaml'):
try:
txt=path.read_text()
except Exception:
continue
if 'kuberture-e2e-edns-public' in txt or 'kuberture-e2e-edns-internal' in txt:
print(f"\nfound job names in {path}")
PYRepository: cozystack/cozystack
Length of output: 2583
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Target file relevant lines:"
sed -n '130,195p' hack/e2e-chainsaw/kuberture/chainsaw-test.yaml
echo
echo "Probes relevant contents:"
sed -n '1,260p' hack/e2e-chainsaw/kuberture/probes.yaml
echo
echo "Kubectl docs/source availability:"
if command -v kubectl >/dev/null 2>&1; then
kubectl logs --help | sed -n '1,120p'
else
echo "kubectl not installed"
fi
echo
echo "Check upstream kubectl help via curl if available:"
python3 - <<'PY'
# Inspect deterministic selector/log behavior described by Kubernetes docs from local kubectl help unavailable.
print("Skipping kubectl docs fetch: no kubectl install available and no network-dependent docs fetch requested.")
PYRepository: cozystack/cozystack
Length of output: 6407
Aggregate logs only from the successful Job pod before making leak-detection assertions.
kubectl logs --max-log-requests=8 -l batch.kubernetes.io/job-name=... reads all pods that still match the Job label, including failed/wedged Job attempts for jobs with backoffLimit: 6 and restartPolicy: Never. Those stale logs can make a completed Job fail the “must NOT see the other hostname” checks. Read logs from the pod selected with status.phase=Succeeded instead.
🤖 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-chainsaw/kuberture/chainsaw-test.yaml` around lines 164 - 175,
Update the log collection in the script to select only pods with
status.phase=Succeeded for each Job before running leak-detection assertions.
Keep the existing Job-name label filters, log options, and separate
pub_logs/int_logs variables, but ensure kubectl logs targets the successful pod
rather than aggregating stale failed attempts.
Andrei Kvapil (kvaps)
left a comment
There was a problem hiding this comment.
LGTM — verified. Probes moved Pod→Job (backoffLimit 6), so a probe that grabs the per-node reserved:ingress IP is recreated on a fresh IP instead of wedging terminally; chainsaw now asserts the Job reaches Complete (fails loud, no false-pass path) and reads logs by the batch.kubernetes.io/job-name label. E2E is green. Closes the IPAM race cleanly.
Bring the worker-pool split branch up to date with main (111 commits), picking up the node-join and CI fixes it was missing: the 18m tenant node-join deadline, guest-fsync-off on ephemeral CI disks (#3455), kubevirt-csi client rate-limit raise (#3428), the Cilium ingress-IP race guard (#3430), and etcd-operator v0.5.4. Conflict resolution: main's #3535 (render the talos-reconcile Job for the default md0 group) modified packages/apps/kubernetes/templates/talos/ talos-reconcile-job.yaml, which this branch deletes because the worker split moves that Job into the kubernetes-nodes chart. Kept the parent template deleted and dropped the parent-scoped test talos_reconcile_nodegroups_test.yaml: the empty-nodeGroups md0-default gap it guards cannot occur in the per-pool child chart, and migration 54 materialises the implicit md0 into an explicit child HelmRelease. targetVersion resolves to 55 (migration 54 on top of main's 53). Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
What this PR does
The split-horizon-routing-proof step ran two external-dns probes as bare Pods and polled them by fixed name. When the Cilium host-scope IPAM plus Gateway API restore race (cilium/cilium#46799) hands a probe the per-node reserved:ingress IP, the sandbox is rejected with "IP already in use" and the e2e leak-healer deletes the wedged pod, expecting a controller to reschedule it onto a free IP. A bare Pod has no controller, so that delete was terminal: the probe vanished and the step failed on a NotFound pod. It surfaces as an intermittent kuberture E2E failure with a
FailedCreatePodSandBoxevent on a probe pod.This runs the probes as Jobs. The Job re-creates its pod on a fresh IP after the healer deletes it, so the proof survives the race. Each Job is asserted to reach Complete instead of polling a fixed pod name, and its logs are read by the job-name label. The failure path also gains a catch podLogs selector for the probe pods, so their stdout is captured when a probe fails for a different reason.
Screenshots
None. No UI changes.
Downstream repositories
The diff touches only the kuberture E2E suite under
hack/e2e-chainsaw/kuberture/. No package, values schema, chart, CRD or platform manifest changed. Nothing in the downstream trigger map is reached.Release note
Summary by CodeRabbit