Skip to content

test(e2e): make kuberture split-horizon probes survive Cilium ingress-IP race - #3430

Merged
Andrei Kvapil (kvaps) merged 1 commit into
mainfrom
test/kuberture-probe-job
Jul 28, 2026
Merged

test(e2e): make kuberture split-horizon probes survive Cilium ingress-IP race#3430
Andrei Kvapil (kvaps) merged 1 commit into
mainfrom
test/kuberture-probe-job

Conversation

@lexfrei

@lexfrei Aleksei Sviridkin (lexfrei) commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

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 FailedCreatePodSandBox event 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

NONE

Summary by CodeRabbit

  • Bug Fixes
    • Improved end-to-end test reliability by hardening the split-horizon routing proof against probe workload churn.
    • Strengthened route isolation validation between public and internal probes.
    • Enhanced failure diagnostics by capturing logs from the completed probe Jobs.
  • Testing
    • Updated DNS EDNS probes to run as Kubernetes Jobs (instead of bare Pods) to support retries and rescheduling.
    • Added explicit assertions that both public and internal probe Jobs reach Complete before running hostname/route isolation checks.

@gemini-code-assist

Copy link
Copy Markdown
Contributor

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

@coderabbitai

coderabbitai Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Changes

EDNS probe routing validation

Layer / File(s) Summary
Job-based EDNS probes
hack/e2e-chainsaw/kuberture/probes.yaml
The public and internal EDNS probes now run as single-shot Kubernetes Jobs with backoffLimit and pod-template labels, while retaining their respective DNS annotation prefixes.
Completion and log capture validation
hack/e2e-chainsaw/kuberture/chainsaw-test.yaml
The test asserts both probe Jobs complete, captures succeeded pod logs by Job-name selectors, and preserves the split-horizon hostname isolation checks. Failure capture also includes probe pods.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Suggested reviewers: androndo, myasnikovdaniil

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
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main E2E change: converting kuberture split-horizon probes to survive the Cilium ingress-IP race.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch test/kuberture-probe-job

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@dosubot dosubot Bot added area/testing Issues or PRs related to testing (e2e, bats, unit tests) kind/flake Categorizes issue or PR as related to a flaky test labels Jul 22, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between 764e304 and ebd0167.

📒 Files selected for processing (2)
  • hack/e2e-chainsaw/kuberture/chainsaw-test.yaml
  • hack/e2e-chainsaw/kuberture/probes.yaml

- --domain-filter=cozystack.test
- --log-level=info
- --log-format=text
backoffLimit: 6

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 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 -S

Repository: 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:


🏁 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))
PY

Repository: 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.

@github-actions github-actions Bot added the size/L This PR changes 100-499 lines, ignoring generated files label Jul 22, 2026
…-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>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (2)
hack/e2e-chainsaw/kuberture/probes.yaml (1)

54-59: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win

Harden 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.0

Also 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 win

Consider failing fast on Failed, not just waiting for Complete.

If a Job exhausts backoffLimit (6) it reaches a Failed condition, not Complete. As written, the assert on Complete will just spin for the full 4m timeout in that case instead of surfacing the real failure quickly. Chainsaw's error: operation can short-circuit this by failing the step immediately if the Failed condition 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

📥 Commits

Reviewing files that changed from the base of the PR and between ebd0167 and 436a19d.

📒 Files selected for processing (2)
  • hack/e2e-chainsaw/kuberture/chainsaw-test.yaml
  • hack/e2e-chainsaw/kuberture/probes.yaml

Comment on lines +164 to +175
- 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)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 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 -S

Repository: 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
done

Repository: 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}")
PY

Repository: 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.")
PY

Repository: 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.

@kvaps Andrei Kvapil (kvaps) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@kvaps
Andrei Kvapil (kvaps) merged commit b22b7b2 into main Jul 28, 2026
17 checks passed
@kvaps
Andrei Kvapil (kvaps) deleted the test/kuberture-probe-job branch July 28, 2026 15:43
IvanHunters added a commit that referenced this pull request Aug 4, 2026
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/testing Issues or PRs related to testing (e2e, bats, unit tests) kind/flake Categorizes issue or PR as related to a flaky test size/L This PR changes 100-499 lines, ignoring generated files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants