Repository navigation
operator mode: add event-driven triggers - #2133
Merged
Merged
Conversation
Diagnose why operator mode feels half-baked (only create-once and cron triggering), map the gap to real user flows (deploy verification, incident triage, remediation), and propose a phased plan headlined by a HealthCheckTrigger CRD for event-driven investigations. Signed-off-by: Claude <noreply@anthropic.com>
…heck Mirror ScheduledHealthCheck (HealthCheckSpec + trigger source, spawns HealthCheck children) rather than a reference to another CRD, since a HealthCheck is an executed instance, not a reusable template. Signed-off-by: Claude <noreply@anthropic.com>
Add an event-driven, self-contained TriggeredHealthCheck CRD (the sibling of ScheduledHealthCheck) that fires an investigation whenever a matching Deployment rolls out a new pod template. - models: TriggeredHealthCheckSpec/Status, DeploymentRolloutTrigger, conditions - trigger_executor: in-memory rollout detection (no kopf annotations on user Deployments), label-selector matching, query-token rendering, rollout settle wait, HealthCheck spawn with ownerReferences, status recording with per- Deployment cooldown and history - handlers/triggeredhealthcheck: validate + Ready condition; low-level Deployment event watcher that fans rollouts out to matching triggers - CRD manifest, RBAC (triggeredhealthchecks + deployments watch) - component tests for helpers, rollout detection, cooldown, and spawn path - docs: new Triggered Health Checks page, nav, index, deployment-verification Reuses the existing HealthCheck execution/history/notification path; each fire spawns a HealthCheck child that becomes the execution record. Signed-off-by: Claude <noreply@anthropic.com>
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
Contributor
📂 Previous Runs
|
| Status | Test case | Time | Turns | Tools | Cost | Total tokens | Input | Max input | Output | Max output | Cached | Non-cached | Reasoning | Compactions | Denied commands | Src |
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| ✅ | 09_crashpod | 31.4s | 4 | 8 | $0.2109 | 69,982 | 68,324 | 19,551 | 1,658 | 694 | 47,961 | 20,363 | 100 | — | — | src |
| ✅ | 101_loki_historical_logs_pod_deleted | 45.2s | 4 | 9 | $0.2357 | 69,878 | 67,313 | 19,683 | 2,565 | 849 | 46,821 | 20,492 | 404 | — | — | src |
| ✅ | 112_find_pvcs_by_uuid | 15.1s | 2 | 2 | $0.1479 | 32,058 | 31,167 | 16,845 | 891 | 546 | 14,319 | 16,848 | 163 | — | — | src |
| ✅ | 12_job_crashing | 37.1s | 5 | 10 | $0.2462 | 93,303 | 91,337 | 20,768 | 1,966 | 518 | 68,438 | 22,899 | 193 | — | — | src |
| ✅ | 176_network_policy_blocking_traffic_no_skills | 30.6s | 4 | 10 | $0.2115 | 67,781 | 66,080 | 19,581 | 1,701 | 515 | 45,457 | 20,623 | 263 | — | — | src |
| ✅ | 227_count_configmaps_per_namespace[0] | 15.4s | 3 | 6 | $0.1490 | 46,347 | 45,644 | 16,659 | 703 | 437 | 28,981 | 16,663 | 29 | — | — | src |
| ✅ | 243_pod_names_contain_service | 30.3s | 3 | 7 | $0.1885 | 49,429 | 47,736 | 18,034 | 1,693 | 655 | 29,233 | 18,503 | 284 | — | — | src |
| ✅ | 24_misconfigured_pvc | 31.8s | 5 | 10 | $0.2220 | 87,046 | 85,359 | 19,000 | 1,687 | 594 | 64,649 | 20,710 | 44 | — | — | src |
| ✅ | 254_elasticsearch_dr_test_log_check | 64.6s | 9 | 14 | $0.3120 | 129,081 | 125,011 | 19,536 | 4,070 | 954 | 103,495 | 21,516 | 244 | — | — | src |
| ✅ | 259_wrong_cluster_logs_confusion | 66.5s | 8 | 11 | $0.2799 | 110,695 | 106,735 | 17,027 | 3,960 | 1,516 | 88,779 | 17,956 | 891 | — | — | src |
| ✅ | 260_global_es_remote_cluster_logs | 62.7s | 5 | 8 | $0.2661 | 72,983 | 68,793 | 17,110 | 4,190 | 1,841 | 50,703 | 18,090 | 990 | — | — | src |
| ✅ | 43_current_datetime_from_prompt | 4.6s | 1 | — | $0.1017 | 14,428 | 14,306 | 14,306 | 122 | 122 | 0 | 14,306 | 78 | — | — | src |
| ✅ | 51_logs_summarize_errors | 20.5s | 3 | 2 | $0.1575 | 47,123 | 46,306 | 17,323 | 817 | 418 | 28,979 | 17,327 | 33 | — | — | src |
| ✅ | 61_exact_match_counting | 8.4s | 2 | 1 | $0.1146 | 29,150 | 28,929 | 14,643 | 221 | 152 | 14,283 | 14,646 | 34 | — | — | src |
| Total | 33.2s avg | 4.1 avg | 7.5 avg | $2.8434 | 919,284 | 893,040 | 20,768 | 26,244 | 1,841 | 632,098 | 260,942 | 3,750 | — | — |
Benchmark Comparison Details
Master baseline: latest master-* experiment (post-merge regression eval)
Status: 14 test/model combinations loaded
- master-27603477607 (created: 2026-06-16)
Benchmark baseline: latest ci-benchmark experiment on master
Status: 17 test/model combinations loaded
- ci-benchmark-27491953079 (created: 2026-06-14)
Time comparison (seconds):
| Test case | This branch | master (4h ago) | Δ vs master | benchmark (2d ago) | Δ vs benchmark |
|---|---|---|---|---|---|
| 09_crashpod (opus-4.6) 📄 | 31.4s | 28.6s | ±0% | — | — |
| 101_loki_historical_logs_pod_deleted (opus-4.6) 📄 | 45.2s | 57.6s | ↓22% | — | — |
| 112_find_pvcs_by_uuid (opus-4.6) 📄 | 15.1s | 14.4s | ±0% | — | — |
| 12_job_crashing (opus-4.6) 📄 | 37.1s | 34.8s | ±0% | — | — |
| 176_network_policy_blocking_traffic_no_skills (opus-4.6) 📄 | 30.6s | 33.5s | ±0% | — | — |
| 227_count_configmaps_per_namespace[0] (opus-4.6) 📄 | 15.4s | 16.2s | ±0% | — | — |
| 243_pod_names_contain_service (opus-4.6) 📄 | 30.3s | 30.1s | ±0% | — | — |
| 24_misconfigured_pvc (opus-4.6) 📄 | 31.8s | 32.9s | ±0% | — | — |
| 254_elasticsearch_dr_test_log_check (opus-4.6) 📄 | 64.6s | 66.3s | ±0% | — | — |
| 259_wrong_cluster_logs_confusion (opus-4.6) 📄 | 66.5s | 74.3s | ↓10% | — | — |
| 260_global_es_remote_cluster_logs (opus-4.6) 📄 | 62.7s | 54.3s | ↑15% | — | — |
| 43_current_datetime_from_prompt (opus-4.6) 📄 | 4.6s | 3.7s | ↑23% | — | — |
| 51_logs_summarize_errors (opus-4.6) 📄 | 20.5s | 18.7s | ±0% | — | — |
| 61_exact_match_counting (opus-4.6) 📄 | 8.4s | 7.8s | ±0% | — | — |
| Total (all, n=14) | 33.2s | 33.8s | — | — | — |
| Comparable (m=14, b=0) | 33.2s | 33.8s | ±0% | — | — |
Cost comparison:
| Test case | This branch | master (4h ago) | Δ vs master | benchmark (2d ago) | Δ vs benchmark |
|---|---|---|---|---|---|
| 09_crashpod (opus-4.6) 📄 | $0.2109 | $0.2078 | ±0% | — | — |
| 101_loki_historical_logs_pod_deleted (opus-4.6) 📄 | $0.2357 | $0.2928 | ↓20% | — | — |
| 112_find_pvcs_by_uuid (opus-4.6) 📄 | $0.1479 | $0.1565 | ±0% | — | — |
| 12_job_crashing (opus-4.6) 📄 | $0.2462 | $0.2440 | ±0% | — | — |
| 176_network_policy_blocking_traffic_no_skills (opus-4.6) 📄 | $0.2115 | $0.2340 | ±0% | — | — |
| 227_count_configmaps_per_namespace[0] (opus-4.6) 📄 | $0.1490 | $0.1485 | ±0% | — | — |
| 243_pod_names_contain_service (opus-4.6) 📄 | $0.1885 | $0.1877 | ±0% | — | — |
| 24_misconfigured_pvc (opus-4.6) 📄 | $0.2220 | $0.2254 | ±0% | — | — |
| 254_elasticsearch_dr_test_log_check (opus-4.6) 📄 | $0.3120 | $0.2901 | ±0% | — | — |
| 259_wrong_cluster_logs_confusion (opus-4.6) 📄 | $0.2799 | $0.3142 | ↓11% | — | — |
| 260_global_es_remote_cluster_logs (opus-4.6) 📄 | $0.2661 | $0.2418 | ↑10% | — | — |
| 43_current_datetime_from_prompt (opus-4.6) 📄 | $0.1017 | $0.1017 | ±0% | — | — |
| 51_logs_summarize_errors (opus-4.6) 📄 | $0.1575 | $0.1571 | ±0% | — | — |
| 61_exact_match_counting (opus-4.6) 📄 | $0.1146 | $0.1146 | ±0% | — | — |
| Total (all, n=14) | $0.2031 | $0.2083 | — | — | — |
| Comparable (m=14, b=0) | $0.2031 | $0.2083 | ±0% | — | — |
Total tokens comparison:
| Test case | This branch | master (4h ago) | Δ vs master | benchmark (2d ago) | Δ vs benchmark |
|---|---|---|---|---|---|
| 09_crashpod (opus-4.6) 📄 | 69,982 | 69,643 | ±0% | — | — |
| 101_loki_historical_logs_pod_deleted (opus-4.6) 📄 | 69,878 | 112,358 | ↓38% | — | — |
| 112_find_pvcs_by_uuid (opus-4.6) 📄 | 32,058 | 33,614 | ±0% | — | — |
| 12_job_crashing (opus-4.6) 📄 | 93,303 | 95,498 | ±0% | — | — |
| 176_network_policy_blocking_traffic_no_skills (opus-4.6) 📄 | 67,781 | 71,617 | ±0% | — | — |
| 227_count_configmaps_per_namespace[0] (opus-4.6) 📄 | 46,347 | 46,324 | ±0% | — | — |
| 243_pod_names_contain_service (opus-4.6) 📄 | 49,429 | 49,466 | ±0% | — | — |
| 24_misconfigured_pvc (opus-4.6) 📄 | 87,046 | 87,845 | ±0% | — | — |
| 254_elasticsearch_dr_test_log_check (opus-4.6) 📄 | 129,081 | 133,188 | ±0% | — | — |
| 259_wrong_cluster_logs_confusion (opus-4.6) 📄 | 110,695 | 118,418 | ±0% | — | — |
| 260_global_es_remote_cluster_logs (opus-4.6) 📄 | 72,983 | 72,135 | ±0% | — | — |
| 43_current_datetime_from_prompt (opus-4.6) 📄 | 14,428 | 14,429 | ±0% | — | — |
| 51_logs_summarize_errors (opus-4.6) 📄 | 47,123 | 47,096 | ±0% | — | — |
| 61_exact_match_counting (opus-4.6) 📄 | 29,150 | 29,149 | ±0% | — | — |
| Total (all, n=14) | 65,663 | 70,056 | — | — | — |
| Comparable (m=14, b=0) | 65,663 | 70,056 | ±0% | — | — |
Cached tokens comparison:
| Test case | This branch | master (4h ago) | Δ vs master | benchmark (2d ago) | Δ vs benchmark |
|---|---|---|---|---|---|
| 09_crashpod (opus-4.6) 📄 | 47,961 | 47,667 | ±0% | — | — |
| 101_loki_historical_logs_pod_deleted (opus-4.6) 📄 | 46,821 | 85,705 | ↓45% | — | — |
| 112_find_pvcs_by_uuid (opus-4.6) 📄 | 14,319 | 14,319 | ±0% | — | — |
| 12_job_crashing (opus-4.6) 📄 | 68,438 | 71,522 | ±0% | — | — |
| 176_network_policy_blocking_traffic_no_skills (opus-4.6) 📄 | 45,457 | 46,821 | ±0% | — | — |
| 227_count_configmaps_per_namespace[0] (opus-4.6) 📄 | 28,981 | 28,979 | ±0% | — | — |
| 243_pod_names_contain_service (opus-4.6) 📄 | 29,233 | 29,245 | ±0% | — | — |
| 24_misconfigured_pvc (opus-4.6) 📄 | 64,649 | 65,561 | ±0% | — | — |
| 254_elasticsearch_dr_test_log_check (opus-4.6) 📄 | 103,495 | 111,091 | ±0% | — | — |
| 259_wrong_cluster_logs_confusion (opus-4.6) 📄 | 88,779 | 93,558 | ±0% | — | — |
| 260_global_es_remote_cluster_logs (opus-4.6) 📄 | 50,703 | 51,153 | ±0% | — | — |
| 43_current_datetime_from_prompt (opus-4.6) 📄 | — | — | — | — | — |
| 51_logs_summarize_errors (opus-4.6) 📄 | 28,979 | 28,923 | ±0% | — | — |
| 61_exact_match_counting (opus-4.6) 📄 | 14,283 | 14,283 | ±0% | — | — |
| Total (all, n=14) | 45,150 | 49,202 | — | — | — |
| Comparable (m=13, b=0) | 48,623 | 52,987 | ±0% | — | — |
Turns comparison:
| Test case | This branch | master (4h ago) | Δ vs master | benchmark (2d ago) | Δ vs benchmark |
|---|---|---|---|---|---|
| 09_crashpod (opus-4.6) 📄 | 4 | 4 | ±0% | — | — |
| 101_loki_historical_logs_pod_deleted (opus-4.6) 📄 | 4 | 6 | ↓33% | — | — |
| 112_find_pvcs_by_uuid (opus-4.6) 📄 | 2 | 2 | ±0% | — | — |
| 12_job_crashing (opus-4.6) 📄 | 5 | 5 | ±0% | — | — |
| 176_network_policy_blocking_traffic_no_skills (opus-4.6) 📄 | 4 | 4 | ±0% | — | — |
| 227_count_configmaps_per_namespace[0] (opus-4.6) 📄 | 3 | 3 | ±0% | — | — |
| 243_pod_names_contain_service (opus-4.6) 📄 | 3 | 3 | ±0% | — | — |
| 24_misconfigured_pvc (opus-4.6) 📄 | 5 | 5 | ±0% | — | — |
| 254_elasticsearch_dr_test_log_check (opus-4.6) 📄 | 9 | 10 | ↓10% | — | — |
| 259_wrong_cluster_logs_confusion (opus-4.6) 📄 | 8 | 8 | ±0% | — | — |
| 260_global_es_remote_cluster_logs (opus-4.6) 📄 | 5 | 5 | ±0% | — | — |
| 43_current_datetime_from_prompt (opus-4.6) 📄 | 1 | 1 | ±0% | — | — |
| 51_logs_summarize_errors (opus-4.6) 📄 | 3 | 3 | ±0% | — | — |
| 61_exact_match_counting (opus-4.6) 📄 | 2 | 2 | ±0% | — | — |
| Total (all, n=14) | 4.1 | 4.4 | — | — | — |
| Comparable (m=14, b=0) | 4.1 | 4.4 | ±0% | — | — |
Tool calls comparison:
| Test case | This branch | master (4h ago) | Δ vs master | benchmark (2d ago) | Δ vs benchmark |
|---|---|---|---|---|---|
| 09_crashpod (opus-4.6) 📄 | 8 | 8 | ±0% | — | — |
| 101_loki_historical_logs_pod_deleted (opus-4.6) 📄 | 9 | 12 | ↓25% | — | — |
| 112_find_pvcs_by_uuid (opus-4.6) 📄 | 2 | 2 | ±0% | — | — |
| 12_job_crashing (opus-4.6) 📄 | 10 | 11 | ±0% | — | — |
| 176_network_policy_blocking_traffic_no_skills (opus-4.6) 📄 | 10 | 11 | ±0% | — | — |
| 227_count_configmaps_per_namespace[0] (opus-4.6) 📄 | 6 | 6 | ±0% | — | — |
| 243_pod_names_contain_service (opus-4.6) 📄 | 7 | 7 | ±0% | — | — |
| 24_misconfigured_pvc (opus-4.6) 📄 | 10 | 11 | ±0% | — | — |
| 254_elasticsearch_dr_test_log_check (opus-4.6) 📄 | 14 | 13 | ±0% | — | — |
| 259_wrong_cluster_logs_confusion (opus-4.6) 📄 | 11 | 12 | ±0% | — | — |
| 260_global_es_remote_cluster_logs (opus-4.6) 📄 | 8 | 8 | ±0% | — | — |
| 43_current_datetime_from_prompt (opus-4.6) 📄 | — | — | — | — | — |
| 51_logs_summarize_errors (opus-4.6) 📄 | 2 | 2 | ±0% | — | — |
| 61_exact_match_counting (opus-4.6) 📄 | 1 | 1 | ±0% | — | — |
| Total (all, n=14) | 7.0 | 8.0 | — | — | — |
| Comparable (m=13, b=0) | 7.5 | 8.0 | ±0% | — | — |
Comparison indicators:
±0%— diff under 10% (within noise threshold)↑N%/↓N%— diff 10-25%↑N%/↓N%— diff over 25% (significant)
📖 Legend
| Icon | Meaning |
|---|---|
| ✅ | The test was successful |
| ➖ | The test was skipped |
| The test failed but is known to be flaky or known to fail | |
| 🚧 | The test had a setup failure (not a code regression) |
| 🔧 | The test failed due to mock data issues (not a code regression) |
| 🚫 | The test was throttled by API rate limits/overload |
| ❌ | The test failed and should be fixed before merging the PR |
🔄 Re-run evals manually
⚠️ Warning:/evalcomments always run using the workflow from master, not from this PR branch. If you modified the GitHub Action (e.g., added secrets or env vars), those changes won't take effect.To test workflow changes, use the GitHub CLI or Actions UI instead:
gh workflow run eval-regression.yaml --repo HolmesGPT/holmesgpt --ref claude/holmesgpt-operator-mode-A2TjD -f markers=regression -f filter=
Option 1: Comment on this PR with /eval:
/eval
tags: regression
Or with more options (one per line):
/eval
model: gpt-4o
tags: regression
id: 09_crashpod
iterations: 5
Run evals on a different branch (e.g., master) for comparison:
/eval
branch: master
tags: regression
| Option | Description |
|---|---|
model |
Model(s) to test (default: same as automatic runs) |
tags |
Pytest tags / markers (no default - runs all tests!) |
id |
Eval ID / pytest -k filter (use /list to see valid eval names) |
iterations |
Number of runs, max 10 |
branch |
Run evals on a different branch (for cross-branch comparison) |
Quick re-run: Use /rerun to re-run the most recent /eval on this PR with the same parameters.
Option 2: Trigger via GitHub Actions UI → "Run workflow"
Option 3: Add PR labels to include extra evals (applies to both automatic runs and /eval comments):
| Label | Effect |
|---|---|
evals-tag-<name> |
Run tests with tag <name> alongside regression |
evals-id-<name> |
Run a specific eval by test ID |
evals-model-<name> |
Override the model (use model list name, e.g. sonnet-4.5) |
Examples: evals-tag-easy, evals-id-09_crashpod, evals-model-sonnet-4.5
🏷️ Valid tags
benchmark, chain-of-causation, compaction, confluence, context_window, conversation_worker, coralogix, counting, database, datadog, datetime, db-connectors, easy, elasticsearch, embeds, fast, frontend, grafana, hard, images, integration, kafka, kubernetes, leaked-information, logs, loki, manual, mcp, medium, metrics, multi-cluster, network, newrelic, no-cicd, numerical, one-test, port-forward, prometheus, question-answer, regression, skills, slackbot, storage, token-limit, toolset-limitation, traces, transparency, victorialogs
🤖 Valid models
deepseek-chat, deepseek-r1-reasoner, deepseek-reasoner, deepseek-v3.2-chat, fable-5, gemini-3-flash-preview, gemini-3-pro-preview, gemini-3.1-pro-preview, gpt-4.1, gpt-5.2-high-reasoning, gpt-5.3-codex, gpt-5.4, gpt-5.5, haiku-4.5, kimi-2.5, kimi-2.5-openrouter, opus-4.5, opus-4.6, opus-4.7, opus-4.8, qwen-next-80B-instruct, qwen-next-80B-thinking, sonnet-4.5, sonnet-4.6
Commands: /eval · /rerun · /list
CLI: gh workflow run eval-regression.yaml --repo HolmesGPT/holmesgpt --ref claude/holmesgpt-operator-mode-A2TjD -f markers=regression -f filter=
Contributor
|
✅ Docker images ready for
Use these tags to pull the images for testing. 📋 Copy commandsgcloud auth configure-docker us-central1-docker.pkg.dev
docker pull us-central1-docker.pkg.dev/robusta-development/temporary-builds/holmes:2d0d02a73
docker tag us-central1-docker.pkg.dev/robusta-development/temporary-builds/holmes:2d0d02a73 me-west1-docker.pkg.dev/robusta-development/development/holmes-dev:2d0d02a73
docker push me-west1-docker.pkg.dev/robusta-development/development/holmes-dev:2d0d02a73
docker pull us-central1-docker.pkg.dev/robusta-development/temporary-builds/holmes-operator:2d0d02a73
docker tag us-central1-docker.pkg.dev/robusta-development/temporary-builds/holmes-operator:2d0d02a73 me-west1-docker.pkg.dev/robusta-development/development/holmes-operator-dev:2d0d02a73
docker push me-west1-docker.pkg.dev/robusta-development/development/holmes-operator-dev:2d0d02a73Patch Helm values in one line (choose the chart you use): HolmesGPT chart: helm upgrade --install holmesgpt ./helm/holmes \
--set registry=me-west1-docker.pkg.dev/robusta-development/development \
--set image=holmes-dev:2d0d02a73 \
--set operator.registry=me-west1-docker.pkg.dev/robusta-development/development \
--set operator.image=holmes-operator-dev:2d0d02a73Robusta wrapper chart: helm upgrade --install robusta robusta/robusta \
--reuse-values \
--set holmes.registry=me-west1-docker.pkg.dev/robusta-development/development \
--set holmes.image=holmes-dev:2d0d02a73 \
--set holmes.operator.registry=me-west1-docker.pkg.dev/robusta-development/development \
--set holmes.operator.image=holmes-operator-dev:2d0d02a73 |
Contributor
|
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:
WalkthroughAdds an alpha TriggeredHealthCheck CRD and operator implementation that watches Deployments for pod-template rollouts, debounces/optionally delays triggers, creates owned HealthCheck resources with templated queries, records trigger history/cooldowns in status, updates RBAC, and adds docs and component tests. ChangesTriggeredHealthCheck Feature Implementation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 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. 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 |
✅ Deploy Preview for holmes-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
helm/holmes/crds/triggeredhealthcheck.yaml (1)
70-76: 💤 Low valueConsider quoting the default value for consistency.
The
default: monitorat line 73 is unquoted, while the enum values at lines 75-76 (alert,monitor) are quoted strings. For consistency and clarity, consider quoting the default asdefault: "monitor".✨ Proposed fix for consistency
mode: type: string description: "Execution mode: 'alert' sends notifications on failure, 'monitor' logs only" - default: monitor + default: "monitor" enum: - alert - monitor🤖 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 `@helm/holmes/crds/triggeredhealthcheck.yaml` around lines 70 - 76, The default value for the CRD field "mode" is unquoted while the enum entries are quoted; update the default under the "mode" schema (field name: mode, property: default) to use a quoted string (default: "monitor") so it matches the quoted enum values ('alert', 'monitor') for consistency and clarity.holmes_operator/handlers/triggeredhealthcheck.py (1)
180-236: ⚡ Quick winConsider optimistic concurrency for condition updates.
The condition update performs a read (line 198-205) and patch (line 225-233) without including
resourceVersionin the patch body. If another process updates the resource between the read and patch, this update may overwrite those changes.While less critical than history/cooldown updates (which use retry logic in
record_trigger), condition updates could still race. Consider either:
- Including
resourceVersionfrom the read (line 199) in the patch body's metadata, similar to lines 399-411 intrigger_executor.py, or- Adding retry-on-409 logic like
_patch_status_with_retry.🤖 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 `@holmes_operator/handlers/triggeredhealthcheck.py` around lines 180 - 236, The set_triggeredhealthcheck_condition function reads the resource and patches its status without optimistic-concurrency control; fetch resource["metadata"]["resourceVersion"] after the get_namespaced_custom_object call and include it as metadata.resourceVersion in the patch body passed to context.k8s_api.patch_namespaced_custom_object_status, or alternatively wrap the patch in retry-on-409 logic similar to _patch_status_with_retry so concurrent updates don't get lost; update the call site that builds body={"status": {"conditions": conditions}} inside set_triggeredhealthcheck_condition accordingly.
🤖 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 `@docs/operator/triggered-health-checks.md`:
- Line 30: Replace the fenced code blocks (the triple-backtick markers like
```yaml and closing ```) with indented code blocks by removing the ``` markers
and prefixing each line of the code block with four spaces (do this for both
occurrences where fenced blocks exist), or alternatively update the project's
markdownlint configuration to allow fenced code blocks (MD046) if fencing was
intentional; target the code fence markers (``` and ```yaml) to locate the
blocks to change.
In `@holmes_operator/trigger_executor.py`:
- Around line 270-332: The code leaves a cooldown unrecorded if record_trigger
fails after the HealthCheck is created; modify settle_and_spawn so the cooldown
is reliably recorded: call and await record_trigger(check_name, namespace,
deployment, old_image, new_image, api=k8s_api) before invoking
k8s_api.create_namespaced_custom_object (or, alternatively, add an idempotency
check using k8s_api.get_namespaced_custom_object to detect an existing
HealthCheck with the same labels generated by
generate_check_name/build_healthcheck_object and skip creation), and if you keep
creation-first, implement a compensation path that deletes the newly created
HealthCheck on record_trigger failure; reference functions/methods:
settle_and_spawn, record_trigger, create_namespaced_custom_object,
build_healthcheck_object, generate_check_name.
---
Nitpick comments:
In `@helm/holmes/crds/triggeredhealthcheck.yaml`:
- Around line 70-76: The default value for the CRD field "mode" is unquoted
while the enum entries are quoted; update the default under the "mode" schema
(field name: mode, property: default) to use a quoted string (default:
"monitor") so it matches the quoted enum values ('alert', 'monitor') for
consistency and clarity.
In `@holmes_operator/handlers/triggeredhealthcheck.py`:
- Around line 180-236: The set_triggeredhealthcheck_condition function reads the
resource and patches its status without optimistic-concurrency control; fetch
resource["metadata"]["resourceVersion"] after the get_namespaced_custom_object
call and include it as metadata.resourceVersion in the patch body passed to
context.k8s_api.patch_namespaced_custom_object_status, or alternatively wrap the
patch in retry-on-409 logic similar to _patch_status_with_retry so concurrent
updates don't get lost; update the call site that builds body={"status":
{"conditions": conditions}} inside set_triggeredhealthcheck_condition
accordingly.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 97259fbc-226b-4e18-aeaf-b44374930782
📒 Files selected for processing (12)
docs/design/2026-06-05_operator-mode-improvements.mddocs/operator/.nav.ymldocs/operator/deployment-verification.mddocs/operator/index.mddocs/operator/triggered-health-checks.mdhelm/holmes/crds/triggeredhealthcheck.yamlhelm/holmes/templates/operator-rbac.yamlholmes_operator/handlers/triggeredhealthcheck.pyholmes_operator/models.pyholmes_operator/operator.pyholmes_operator/trigger_executor.pytests/holmes_operator/test_triggeredhealthcheck_component.py
…fe scheduling Add a deliberate post-rollout delay (delaySeconds), distinct from settleTimeout: settleTimeout waits for the rollout to become available; delaySeconds waits a chosen period (minutes to days) so slow-burn regressions (leaks, pool exhaustion) can surface before Holmes evaluates. Delayed fires are persisted in status.pending and reconciled by a kopf timer, so they survive operator restarts (verified end-to-end: a pending fire ran correctly after killing and restarting the operator mid-delay). Pending entries are claimed before spawning (no double-run) and debounced per Deployment (a newer rollout replaces a still-pending one). - models: delaySeconds field, PendingCheck, status.pending - trigger_executor: compute_fire_at, due_pending, add_pending (debounced), remove_pending - handler: delay branch in the rollout watcher + on.timer reconciler - CRD: delaySeconds spec field, status.pending schema - tests: pending-queue add/remove/due/debounce Validated end-to-end against a real k3s cluster with the real Holmes API driving an LLM investigation (kubectl queries -> pass/fail verdict in the HealthCheck). Signed-off-by: Claude <noreply@anthropic.com>
Run rollout checks 5 minutes after the rollout by default so transient post-deploy issues have time to surface. Set delaySeconds: 0 to check as soon as the rollout settles. Signed-off-by: Claude <noreply@anthropic.com>
aantn
changed the title
The TriggeredHealthCheck design has shipped; the proposal doc is no longer needed. Signed-off-by: Claude <noreply@anthropic.com>
The two waits were framed as either/or alternatives and the run order was described backwards. Rewrite around the actual behaviour: a rollout schedules the check after a fixed delaySeconds soak (persisted, survives restart), then a settleTimeout-capped wait for the rollout to become Available runs just before the check. Add a timeline, a fixed-vs-capped comparison table, the meaning of 0 for each, concrete example timings, and disambiguate the two restart concerns. Signed-off-by: Claude <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/operator/triggered-health-checks.md (1)
33-57:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd a default Claude 4.5 model in the primary manifest example.
The example is missing
spec.model, so it doesn’t follow the docs rule to default reference examples to the latest Claude 4.5 model. Add, for example,model: anthropic/claude-sonnet-4-5-20250929.As per coding guidelines, “Primary documentation examples should use the latest Anthropic Claude models …” and “Reference documentation examples should use the latest Claude 4.5 models …”.
🤖 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 `@docs/operator/triggered-health-checks.md` around lines 33 - 57, The example TriggeredHealthCheck manifest is missing spec.model; add a default Claude 4.5 model entry under spec (e.g., set spec.model to "anthropic/claude-sonnet-4-5-20250929") in the primary YAML example so the TriggeredHealthCheck resource includes the required model reference and follows the docs guideline for using the latest Claude 4.5 model.Source: Coding guidelines
♻️ Duplicate comments (1)
docs/operator/triggered-health-checks.md (1)
103-106:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix the timeline block lint violations (language + fence style).
Line 103 uses a fenced block without language (MD040), and current lint also expects indented style (MD046). Please either convert this timeline to the repository’s expected style or update lint config consistently.
Based on learnings from prior review context, a similar MD046 issue was already flagged for this file and should be handled consistently.
🤖 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 `@docs/operator/triggered-health-checks.md` around lines 103 - 106, Replace the fenced code block timeline in triggered-health-checks.md with the repository-expected indented code block style to satisfy MD046 (remove the ``` fence and indent each timeline line by four spaces) and ensure no language tag is added (MD040); keep the exact timeline content (the ASCII arrows and labels: "rollout detected ──► wait delaySeconds ──► wait for rollout to be Available ──► check runs" and the two parenthetical lines) so the lint rule passes consistently with the prior MD046 fix in this file.Source: Linters/SAST tools
🤖 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.
Outside diff comments:
In `@docs/operator/triggered-health-checks.md`:
- Around line 33-57: The example TriggeredHealthCheck manifest is missing
spec.model; add a default Claude 4.5 model entry under spec (e.g., set
spec.model to "anthropic/claude-sonnet-4-5-20250929") in the primary YAML
example so the TriggeredHealthCheck resource includes the required model
reference and follows the docs guideline for using the latest Claude 4.5 model.
---
Duplicate comments:
In `@docs/operator/triggered-health-checks.md`:
- Around line 103-106: Replace the fenced code block timeline in
triggered-health-checks.md with the repository-expected indented code block
style to satisfy MD046 (remove the ``` fence and indent each timeline line by
four spaces) and ensure no language tag is added (MD040); keep the exact
timeline content (the ASCII arrows and labels: "rollout detected ──► wait
delaySeconds ──► wait for rollout to be Available ──► check runs" and the two
parenthetical lines) so the lint rule passes consistently with the prior MD046
fix in this file.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5acef14a-4f8f-44b5-ae4e-796cd66a4bf7
📒 Files selected for processing (1)
docs/operator/triggered-health-checks.md
…s knob
Remove settleTimeout. Having two interacting waits (a cap on waiting for the
rollout to become Available, plus a fixed delay) was hard to understand. Collapse
to one plain-English field:
delaySeconds = how long to wait after a rollout before running the check
(default 5 min; 0 = immediately; up to 7 days)
If the wait is shorter than the rollout, the check just reports that the rollout
hasn't finished yet, so no separate settle logic is needed. Drop wait_for_rollout
and the settle step; rename settle_and_spawn -> spawn_check.
Also make TriggeredHealthCheck the primary, first-class example on the deployment
verification page (declare-once auto-verification), keeping the inline HealthCheck
pattern for synchronous CI/CD gating, with a when-to-use-which table.
Verified end-to-end on a live k3s cluster with the real Holmes API + LLM: default
300 schedules the check 5 min out; a short delay fires through the timer into a
real investigation (kubectl query -> pass verdict in the HealthCheck).
Signed-off-by: Claude <noreply@anthropic.com>
Previously the spawned HealthCheck only knew the rollout facts if the author
wired the {{ .deployment }}/{{ .namespace }}/{{ .old.image }}/{{ .new.image }}
tokens into their query; a terse query like 'Is it healthy?' gave the model no
deployment or namespace to work with. Now a structured context header (deployment,
namespace, previous/new image) is always prepended to the query, so the model has
the facts regardless of how the query is written. Tokens still work for inline use.
Verified end-to-end on a live k3s cluster with Opus 4.8 via OpenRouter using a
terse query that never named the namespace: broken rollout (bad image) -> fail
citing the ImagePullBackOff pod; healthy rollout -> pass. Server logs confirm the
model queried the correct namespace purely from the injected context.
Signed-off-by: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
operator mode improvements
Summary by CodeRabbit
New Features
Documentation