POC for running evals live in CI/CD - #844
Conversation
WalkthroughAdds KIND-based Kubernetes setup to CI and enables live tests; replaces fixed sleeps with readiness + polling in two Kubernetes test fixtures; records per-test-type setup-failure counts in GitHub reporting; and treats setup failures as non-regressions in test-results logic. Changes
Sequence Diagram(s)sequenceDiagram
participant GH as GitHub Actions
participant CI as llm-evaluation workflow
participant Tools as kind/kubectl
participant Cluster as KIND Cluster
participant Tests as pytest
GH->>CI: trigger workflow
CI->>Tools: install kubectl & kind
CI->>CI: write kind-config.yaml
CI->>Tools: kind create cluster --config kind-config.yaml --wait
Tools->>Cluster: provision nodes & apply patches
CI->>Cluster: wait for nodes & kube-system pods Ready
CI->>Tests: set RUN_LIVE and run pytest (-n10 -s)
Tests->>Cluster: run fixtures (readiness + OOM/crash polling)
Tests->>CI: return test results (include is_setup_failure flags)
CI->>GH: generate GitHub report (includes setup-failure counts)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
.github/workflows/llm-evaluation.yaml (1)
101-101: RUN_LIVE=true on PRs may incur external API spend; gate live evalsEnabling live evals by default can trigger paid API calls (OpenAI/Azure/Braintrust). Secrets are not available for forked PRs, which may fail or lead to confusing behavior.
Consider gating the test step to run live only on trusted contexts (push to master, workflow_dispatch, or labeled PRs). Example snippet (outside the current diff range) to add to the “Run tests” step:
if: > github.event_name == 'push' && github.ref == 'refs/heads/master' || github.event_name == 'workflow_dispatch' || (github.event_name == 'pull_request' && contains(join(fromJson(toJson(github.event.pull_request.labels)).*.name, ','), 'run-live'))Would you like me to open a follow-up PR to implement this gating and add a “run-live” label workflow?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (1)
.github/workflows/llm-evaluation.yaml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Pre-commit checks
- GitHub Check: llm_evals
- GitHub Check: Pre-commit checks
🔇 Additional comments (1)
.github/workflows/llm-evaluation.yaml (1)
55-63: Validate the need for hostPort mappings 30000/30001; potential port conflictsMapping NodePort ranges to host ports is only needed if tests hit NodePort services from the host. On shared CI runners, fixed host ports can conflict with other processes.
If NodePorts are not required, drop extraPortMappings to reduce surface area. If they are required, consider using unique, less common ports or dynamically deriving them from env to avoid conflicts. I can propose a parametric config if needed.
Also applies to: 59-63
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
tests/llm/fixtures/test_ask_holmes/02_what_is_wrong_with_pod/test_case.yaml (1)
23-31: The current command is unlikely to OOM; switch to a deterministic memory hog.
tail /dev/zerotypically does not grow process memory and may never trigger OOMKill, causing flaky timeouts. Use an in-shell memory amplification loop that reliably consumes memory within the container’s limit.Apply this diff to make OOM conditions deterministic:
containers: - name: main image: busybox:1.35 - command: ["sh", "-c", "tail /dev/zero"] + # Reliably consume memory until the container hits its limit and gets OOMKilled + command: ["sh", "-c", "X='x'; while true; do X=$X$X$X$X; done"] resources: limits: memory: "50Mi" requests: memory: "50Mi"
🧹 Nitpick comments (2)
tests/llm/fixtures/test_ask_holmes/02_what_is_wrong_with_pod/test_case.yaml (2)
35-48: Avoid logs that disclose the expected root cause; keep phrasing neutral.To comply with test hygiene, don’t echo messages that reveal the expected failure mode.
Apply this diff to neutralize messages:
- echo "Waiting for pod to be OOMKilled..." + echo "Waiting for pod lifecycle transition..." @@ - if [ "$STATUS" = "OOMKilled" ]; then - echo "Pod was OOMKilled successfully" + if [ "$STATUS" = "OOMKilled" ]; then + echo "Detected pod termination" @@ - echo "Timeout waiting for OOMKill after 120 seconds" + echo "Timeout waiting for expected condition after 120 seconds"
12-31: Optional: explicitly set restartPolicy to Always for clarity.Pods default to
restartPolicy: Always, but being explicit makes the OOM/restart loop intention unambiguous.Apply this diff:
spec: - containers: + restartPolicy: Always + containers:
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (4)
.github/workflows/llm-evaluation.yaml(1 hunks)tests/llm/fixtures/test_ask_holmes/02_what_is_wrong_with_pod/test_case.yaml(1 hunks)tests/llm/utils/reporting/github_reporter.py(6 hunks)tests/llm/utils/test_results.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/llm-evaluation.yaml
🧰 Additional context used
📓 Path-based instructions (4)
**/*.py
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.py: ALWAYS place Python imports at the top of the file, not inside functions or methods
Use Ruff for formatting and linting (configured in pyproject.toml)
Type hints required (mypy configuration in pyproject.toml)
Pre-commit hooks enforce quality checks
Don't add convenience logs that give away the problem
Don't write logs that directly state the issue
Ensure historical timestamps are properly handled in logs (especially with Loki)
Files:
tests/llm/utils/test_results.pytests/llm/utils/reporting/github_reporter.py
tests/**
📄 CodeRabbit Inference Engine (CLAUDE.md)
Tests must match source structure under tests/
Files:
tests/llm/utils/test_results.pytests/llm/utils/reporting/github_reporter.pytests/llm/fixtures/test_ask_holmes/02_what_is_wrong_with_pod/test_case.yaml
**/*.yaml
📄 CodeRabbit Inference Engine (CLAUDE.md)
ALWAYS use Secrets for scripts, not inline manifests or ConfigMaps (prevents code visibility with kubectl describe)
Files:
tests/llm/fixtures/test_ask_holmes/02_what_is_wrong_with_pod/test_case.yaml
tests/**/*.yaml
📄 CodeRabbit Inference Engine (CLAUDE.md)
tests/**/*.yaml: Never use names that hint at the problem or expected behavior in resource names (e.g., avoid 'broken-pod', 'test-project-that-does-not-exist', 'crashloop-app'). Use neutral names that don't give away what the LLM should discover
Each test must use a dedicated namespace app- to prevent conflicts
All pod names must be unique across tests
Resource naming should be neutral, not hint at the problem
Use minimal resource footprints (e.g., reduce memory/CPU for Loki in tests)
Files:
tests/llm/fixtures/test_ask_holmes/02_what_is_wrong_with_pod/test_case.yaml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: llm_evals
- GitHub Check: Pre-commit checks
- GitHub Check: Pre-commit checks
🔇 Additional comments (5)
tests/llm/utils/test_results.py (1)
76-81: Setup failures should not be treated as regressions — good change.Early-returning False for is_setup_failure aligns reporting with intent and avoids skewing regression counts.
tests/llm/utils/reporting/github_reporter.py (4)
34-34: Initialize per-suite setup-failure counters — looks correct.Counters are clearly named and scoped per test suite.
Also applies to: 41-41, 48-48
57-58: Classifying setup failures before pass/regression/mock is the right precedence.This ensures setup issues don’t inflate pass/fail or regression counts.
Also applies to: 69-70, 81-82
95-96: Including setup-failure counts in the summary lines is helpful.The appended “setup failures” fragment is consistent across suites and matches the counters.
Also applies to: 104-105, 114-115
143-146: Legend entry for setup failures improves clarity.The new 🚧 legend item sets the right expectation for non-code regressions.
…est_case.yaml Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…olmesgpt into better-ci-cd-evals
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
tests/llm/fixtures/test_ask_holmes/02_what_is_wrong_with_pod/test_case.yaml (1)
25-25: This workload likely won’t OOM; tail /dev/zero uses constant memory.tail reads from /dev/zero with a fixed-size buffer and streams to stdout; it doesn’t grow memory usage toward the 50Mi limit. The test may time out waiting for OOMKilled.
Replace the command with a small memory-hogging loop that grows a shell variable until the container hits its memory limit.
Apply this diff:
- command: ["sh", "-c", "tail /dev/zero"] + command: ["sh", "-c", "s=0123456789abcdef; while true; do s=$s$s$s$s$s$s$s$s; sleep 0.05; done"]
♻️ Duplicate comments (1)
tests/llm/fixtures/test_ask_holmes/02_what_is_wrong_with_pod/test_case.yaml (1)
33-33: Previous suggestion incorporated: readiness wait made non-fatal.Appending
|| trueprevents early exit if the pod never becomes Ready (e.g., OOMs immediately). Thanks for addressing this.
🧹 Nitpick comments (3)
tests/llm/fixtures/test_ask_holmes/02_what_is_wrong_with_pod/test_case.yaml (3)
37-41: Broaden OOM detection to include current state.terminated.reason.In some intervals, lastState may be empty while state.terminated.reason is populated (or vice versa). Check both to reduce flakiness.
Apply this diff:
- STATUS=$(kubectl get pod giant-narwhal-6958c5bdd8-69gtn -n app-02 -o jsonpath='{.status.containerStatuses[0].lastState.terminated.reason}' 2>/dev/null || echo "") - if [ "$STATUS" = "OOMKilled" ]; then + STATUS_LAST=$(kubectl get pod giant-narwhal-6958c5bdd8-69gtn -n app-02 -o jsonpath='{.status.containerStatuses[0].lastState.terminated.reason}' 2>/dev/null || echo "") + STATUS_CURR=$(kubectl get pod giant-narwhal-6958c5bdd8-69gtn -n app-02 -o jsonpath='{.status.containerStatuses[0].state.terminated.reason}' 2>/dev/null || echo "") + if [ "$STATUS_LAST" = "OOMKilled" ] || [ "$STATUS_CURR" = "OOMKilled" ]; then echo "Pod was OOMKilled successfully" break fi
36-48: Make the polling loop POSIX-shell friendly to avoid brace expansion dependency.Some CI shells don’t support
{1..60}. Use a while loop for portability.Apply this diff:
- for i in {1..60}; do + i=1 + while [ "$i" -le 60 ]; do @@ - if [ $i -eq 60 ]; then + if [ "$i" -eq 60 ]; then echo "Timeout waiting for OOMKill after 120 seconds" kubectl describe pod giant-narwhal-6958c5bdd8-69gtn -n app-02 exit 1 fi sleep 2 - done + i=$((i+1)) + done
50-51: Make teardown idempotent and faster.Ignore not-found errors and don’t block on full namespace deletion to speed up CI and avoid masking prior failures.
Apply this diff:
- kubectl delete namespace app-02 + kubectl delete namespace app-02 --ignore-not-found --wait=false
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (2)
.github/workflows/llm-evaluation.yaml(2 hunks)tests/llm/fixtures/test_ask_holmes/02_what_is_wrong_with_pod/test_case.yaml(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/llm-evaluation.yaml
🧰 Additional context used
📓 Path-based instructions (3)
tests/**
📄 CodeRabbit Inference Engine (CLAUDE.md)
Tests must match source structure under tests/
Files:
tests/llm/fixtures/test_ask_holmes/02_what_is_wrong_with_pod/test_case.yaml
**/*.yaml
📄 CodeRabbit Inference Engine (CLAUDE.md)
ALWAYS use Secrets for scripts, not inline manifests or ConfigMaps (prevents code visibility with kubectl describe)
Files:
tests/llm/fixtures/test_ask_holmes/02_what_is_wrong_with_pod/test_case.yaml
tests/**/*.yaml
📄 CodeRabbit Inference Engine (CLAUDE.md)
tests/**/*.yaml: Never use names that hint at the problem or expected behavior in resource names (e.g., avoid 'broken-pod', 'test-project-that-does-not-exist', 'crashloop-app'). Use neutral names that don't give away what the LLM should discover
Each test must use a dedicated namespace app- to prevent conflicts
All pod names must be unique across tests
Resource naming should be neutral, not hint at the problem
Use minimal resource footprints (e.g., reduce memory/CPU for Loki in tests)
Files:
tests/llm/fixtures/test_ask_holmes/02_what_is_wrong_with_pod/test_case.yaml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: llm_evals
- GitHub Check: Pre-commit checks
- GitHub Check: Pre-commit checks
🔇 Additional comments (2)
tests/llm/fixtures/test_ask_holmes/02_what_is_wrong_with_pod/test_case.yaml (2)
1-1: Good: prompt now targets the correct namespace.Including "namespace": "app-02" aligns the prompt with the isolated test namespace requirement.
17-21: Resource naming and scoping look compliant.Neutral pod name, unique namespace app-02, and minimal resource limits meet the test YAML guidelines.
Confirm that pod names are unique across all tests, not just within their namespaces, per the guideline. If cross-test uniqueness isn’t guaranteed, namespace scoping probably mitigates conflicts, but we can systematically check repo-wide if needed.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
.github/workflows/llm-evaluation.yaml (4)
34-39: Pin kubectl and verify checksum to prevent supply-chain risk and client/server skewUsing the moving "stable" tag with no checksum makes builds non-reproducible and insecure. Pin a version aligned with your KIND node image and verify its SHA256; alternatively, use a maintained setup action.
Apply this patch:
- - name: Install kubectl - run: | - curl -LO "https://dl.k8s.io/release/$(curl -L -s https://dl.k8s.io/release/stable.txt)/bin/linux/amd64/kubectl" - chmod +x kubectl - sudo mv kubectl /usr/local/bin/ - kubectl version --client + - name: Install kubectl + run: | + set -euo pipefail + # Pin kubectl; align with KIND node image to avoid skew + KUBECTL_VERSION="${KUBECTL_VERSION:-v1.28.9}" + curl -fsSLo kubectl "https://dl.k8s.io/release/${KUBECTL_VERSION}/bin/linux/amd64/kubectl" + curl -fsSLo kubectl.sha256 "https://dl.k8s.io/release/${KUBECTL_VERSION}/bin/linux/amd64/kubectl.sha256" + echo "$(cat kubectl.sha256) kubectl" | sha256sum --check --status + chmod +x kubectl + sudo mv kubectl /usr/local/bin/ + kubectl version --client=trueAlternative (simpler and pinned):
- uses: azure/setup-kubectl@v4
with:
version: v1.28.9
41-46: Verify KIND binary integrity and keep version pin explicitDownload includes no checksum verification. Add SHA256 verification and keep the version pin explicit for reproducibility; or use a maintained action.
- - name: Install KIND - run: | - curl -Lo ./kind https://kind.sigs.k8s.io/dl/v0.20.0/kind-linux-amd64 - chmod +x ./kind - sudo mv ./kind /usr/local/bin/kind - kind version + - name: Install KIND + run: | + set -euo pipefail + KIND_VERSION="${KIND_VERSION:-v0.20.0}" + curl -fsSLo kind "https://kind.sigs.k8s.io/dl/${KIND_VERSION}/kind-linux-amd64" + curl -fsSLo kind.sha256sum "https://kind.sigs.k8s.io/dl/${KIND_VERSION}/kind-linux-amd64.sha256sum" + # The checksum file references 'kind-linux-amd64' as the filename; adapt for local name 'kind' + awk '{print $1" kind"}' kind.sha256sum | sha256sum --check --status + chmod +x ./kind + sudo mv ./kind /usr/local/bin/kind + kind versionAlternative: use helm/kind-action@v1 with with.version: v0.20.0 to avoid manual install and gain built-in waiting.
48-85: kubeadm patches missing apiVersion and duplicate max-pods settingsEach kubeadmConfigPatches document should include apiVersion. Also, you set max pods twice (kubeletExtraArgs.max-pods and KubeletConfiguration.maxPods). Keep one (prefer KubeletConfiguration.maxPods) to avoid conflicts.
- - name: Create KIND cluster - run: | - cat <<EOF > kind-config.yaml + - name: Create KIND cluster + run: | + set -euo pipefail + cat <<-EOF > kind-config.yaml kind: Cluster apiVersion: kind.x-k8s.io/v1alpha4 nodes: - role: control-plane extraPortMappings: - containerPort: 30000 hostPort: 30000 protocol: TCP - containerPort: 30001 hostPort: 30001 protocol: TCP kubeadmConfigPatches: - | - kind: InitConfiguration - nodeRegistration: - kubeletExtraArgs: - max-pods: "200" + apiVersion: kubeadm.k8s.io/v1beta3 + kind: InitConfiguration - | - kind: KubeProxyConfiguration + apiVersion: kubeproxy.config.k8s.io/v1alpha1 + kind: KubeProxyConfiguration metricsBindAddress: "0.0.0.0:10249" - | - kind: KubeletConfiguration + apiVersion: kubelet.config.k8s.io/v1beta1 + kind: KubeletConfiguration maxPods: 200 - role: worker kubeadmConfigPatches: - | - kind: JoinConfiguration - nodeRegistration: - kubeletExtraArgs: - max-pods: "200" + apiVersion: kubeadm.k8s.io/v1beta3 + kind: JoinConfiguration - | - kind: KubeletConfiguration + apiVersion: kubelet.config.k8s.io/v1beta1 + kind: KubeletConfiguration maxPods: 200 EOFNote: Please align apiVersion values with the Kubernetes version of your kind node image (suggested below). If you prefer kubeletExtraArgs, drop KubeletConfiguration instead—don’t set both.
97-100: Replace “wait for all kube-system pods” with targeted rollout checks to avoid hangs/flakesWaiting on all pods can hang on Jobs/DaemonSets and is brittle. Use rollout for core components (coredns, kube-proxy, kindnet).
- # Wait for all system pods to be ready - echo "Waiting for system pods to be ready..." - kubectl wait --for=condition=Ready pods --all -n kube-system --timeout=300s + # Wait for core components to be ready + echo "Waiting for core system components to be ready..." + kubectl -n kube-system rollout status deployment/coredns --timeout=300s + kubectl -n kube-system rollout status daemonset/kube-proxy --timeout=300s || true + # Kind’s default CNI is kindnet; ignore if different + kubectl -n kube-system rollout status daemonset/kindnet --timeout=300s || true
🧹 Nitpick comments (3)
.github/workflows/llm-evaluation.yaml (3)
108-114: Add more cluster state for debugging failures (optional)Consider printing kube-system events on failure to speed up triage.
For example:
- kubectl get events -A --sort-by=.lastTimestamp | tail -n 200
- kubectl -n kube-system describe ds/kube-proxy || true
131-131: Use xdist auto worker count to avoid oversubscription on 2 vCPU runners-n10 can lead to heavy context switching and longer wall time on GitHub’s 2-core runners. Let xdist auto-detect or cap to 2–3.
- poetry run pytest --no-cov tests/llm/test_ask_holmes.py tests/llm/test_investigate.py -s -n10 -m 'llm and easy' + poetry run pytest --no-cov tests/llm/test_ask_holmes.py tests/llm/test_investigate.py -s -nauto -m 'llm and easy'
86-91: Pin KIND node image and align kubectl versionPin the kind node image and add a kubectl version check to avoid client/server skew; I couldn't verify whether these vars are already defined elsewhere—please confirm across workflows.
- File: .github/workflows/llm-evaluation.yaml (approx. lines 86-91)
- # Create cluster with resource limits appropriate for GitHub Actions (7GB RAM, 2 CPU) - kind create cluster --config kind-config.yaml --wait 5m + # Create cluster (GitHub-hosted runners typically have ~7GB RAM, 2 vCPU) + KIND_NODE_IMAGE="${KIND_NODE_IMAGE:-kindest/node:v1.28.9}" + kind create cluster --config kind-config.yaml --image "$KIND_NODE_IMAGE" --wait 5m @@ - # Configure kubectl - kubectl cluster-info --context kind-kind + # Configure kubectl / verify access + kubectl cluster-info --context kind-kind + kubectl version --shortRecommend setting KUBECTL_VERSION=v1.28.9 to match KIND_NODE_IMAGE=v1.28.9 and confirm the intended Kubernetes minor version across your CI.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (1)
.github/workflows/llm-evaluation.yaml(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: llm_evals
- GitHub Check: Pre-commit checks
- GitHub Check: Pre-commit checks
🔇 Additional comments (1)
.github/workflows/llm-evaluation.yaml (1)
120-120: RUN_LIVE enabled in CI — confirm test idempotency and rate limitsEnabling live runs in CI can incur API costs and rate limits. Ensure retries/backoff are in place and secrets are scoped. If this is intentional for the POC, LGTM.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
tests/llm/fixtures/test_ask_holmes/18_crash_looping_v2/test_case.yaml (5)
6-6: Pin the remote manifest to a specific commit (avoid drift, improve reproducibility)Fetching k8s manifests from a moving target (main) can break tests nondeterministically and is a supply-chain risk. Pin to a commit SHA and reuse the same URL in cleanup. Also, deleting the namespace is sufficient for cleanup; the explicit delete of the manifest is redundant.
Apply this diff:
- kubectl apply -f https://raw.githubusercontent.com/robusta-dev/kubernetes-demos/main/crashpod.v2/crashloop-cert-app.yaml -n app-18 + REPO="robusta-dev/kubernetes-demos" + COMMIT="<pin-specific-commit-sha>" + PATH="crashpod.v2/crashloop-cert-app.yaml" + URL="https://raw.githubusercontent.com/${REPO}/${COMMIT}/${PATH}" + kubectl apply -f "$URL" -n app-18- kubectl delete -f https://raw.githubusercontent.com/robusta-dev/kubernetes-demos/main/crashpod.v2/crashloop-cert-app.yaml -n app-18 - kubectl delete namespace app-18 + kubectl delete namespace app-18 --wait=falseAlso applies to: 29-30
12-15: Capture terminated state as well (covers fast crash scenarios)Some containers crash so quickly that
.state.waiting.reasonmay be empty while.lastState.terminated.reasonis set to "Error". Capture that, too.Apply this diff:
- waiting_reason=$(kubectl get pods -n app-18 -l app=flask -o jsonpath='{.items[0].status.containerStatuses[0].state.waiting.reason}' 2>/dev/null || echo "") + waiting_reason=$(kubectl get pods -n app-18 -l app=flask -o jsonpath='{.items[0].status.containerStatuses[0].state.waiting.reason}' 2>/dev/null || echo "") + terminated_reason=$(kubectl get pods -n app-18 -l app=flask -o jsonpath='{.items[0].status.containerStatuses[0].lastState.terminated.reason}' 2>/dev/null || echo "")
16-20: Tighten success condition to reduce false positivesA single restart might happen transiently. Consider treating CrashLoopBackOff or terminated=Error or restartCount >= 2 as the success criteria. Also include terminated_reason in the message for better diagnostics.
Apply this diff:
- if [ "$waiting_reason" = "CrashLoopBackOff" ] || [ "$restart_count" -gt "0" ]; then - echo "Pod has crashed (status: $waiting_reason, restarts: $restart_count)" + if [ "$waiting_reason" = "CrashLoopBackOff" ] || [ "$terminated_reason" = "Error" ] || [ "$restart_count" -ge "2" ]; then + echo "Pod has crashed (waiting: $waiting_reason, terminated: $terminated_reason, restarts: $restart_count)" exit 0 fi
6-6: Potential naming hint via external manifest; ensure resource names are neutralRepository guidelines: “Never use names that hint at the problem in resource names.” The external file path includes “crashloop-cert-app.yaml”, which suggests the failure mode. If the manifest defines resources with hinting names, the LLM may see them.
Please verify the actual resource names created by that manifest (e.g., kubectl -n app-18 get all -o name) do not contain “crashloop” or similar hints. If they do, consider:
- Vendoring a copy in tests with neutral resource names; or
- Applying an overlay (e.g., kustomize) to rename resources to neutral names before applying.
Would you like a small overlay example that rewrites metadata.name and labels to neutral terms?
8-27: Minor: make timeout configurable via env for CI tuningAllow overriding the 360s timeout via an env var to balance speed vs. flakiness across environments.
Apply this diff:
- timeout=360 + timeout="${CRASH_WAIT_TIMEOUT:-360}"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
tests/llm/fixtures/test_ask_holmes/18_crash_looping_v2/test_case.yaml(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
tests/**
📄 CodeRabbit Inference Engine (CLAUDE.md)
Tests must match source structure under tests/
Files:
tests/llm/fixtures/test_ask_holmes/18_crash_looping_v2/test_case.yaml
**/*.yaml
📄 CodeRabbit Inference Engine (CLAUDE.md)
ALWAYS use Secrets for scripts, not inline manifests or ConfigMaps (prevents code visibility with kubectl describe)
Files:
tests/llm/fixtures/test_ask_holmes/18_crash_looping_v2/test_case.yaml
tests/**/*.yaml
📄 CodeRabbit Inference Engine (CLAUDE.md)
tests/**/*.yaml: Never use names that hint at the problem or expected behavior in resource names (e.g., avoid 'broken-pod', 'test-project-that-does-not-exist', 'crashloop-app'). Use neutral names that don't give away what the LLM should discover
Each test must use a dedicated namespace app- to prevent conflicts
All pod names must be unique across tests
Resource naming should be neutral, not hint at the problem
Use minimal resource footprints (e.g., reduce memory/CPU for Loki in tests)
Files:
tests/llm/fixtures/test_ask_holmes/18_crash_looping_v2/test_case.yaml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Pre-commit checks
🔇 Additional comments (1)
tests/llm/fixtures/test_ask_holmes/18_crash_looping_v2/test_case.yaml (1)
5-7: Good move: per-test namespace + polling over fixed sleepsUsing a dedicated namespace (app-18) and replacing fixed sleeps with a bounded polling loop increases determinism and reduces flakiness in CI.
2 participants
No description provided.