feat: add standalone promtool validation scripts - #41
Conversation
|
Preview environment torn down. |
|
Warning Review limit reached
Next review available in: 34 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a Promtool validation pipeline for Helm alert charts: ChangesPromtool Validation Pipeline
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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 `@sample/promtool-tests/mariadb-alerts/mariadb_alerts_test.yaml`:
- Around line 14-30: Add missing test coverage for the
MariadbLatencySlowQueries_WarnThreshold alert in mariadb_alerts_test.yaml.
Update the alert_rule_test section to include an assertion for this alert
alongside the existing MariadbLatencySlowQueries_CriticalThreshold and
MariadbTrafficQpsHigh_MaxQps cases, using the same pattern for eval_time,
exp_alerts, exp_labels, and exp_annotations so the warning slow-query rule is
exercised by the sample suite.
In `@scripts/extract_rules.py`:
- Around line 103-105: main() is treating an explicitly passed empty argv the
same as no argv, so callers of main([]) are incorrectly falling back to
sys.argv[1:]. Update main in extract_rules.py to distinguish None from an empty
list by only using sys.argv[1:] when argv is None, and otherwise pass the
provided argv directly into parse_args.
- Around line 34-40: The validation in extract_rules.py is masking malformed
PrometheusRule shapes by using defaults for spec and groups. Update the logic
around document.get("spec") in the rule extraction path so invalid empty or
wrong-typed values like spec: [] or groups: {} still trigger RuleExtractionError
instead of being coerced to {} or []. Keep the existing type checks in the
extraction flow, especially the code that handles spec and rule_groups, but
remove the fallback behavior that turns bad shapes into empty containers.
In `@scripts/render_and_check.py`:
- Around line 52-63: The skip logic in should_skip_deployment_chart is too broad
because it excludes any Chart.yaml whose path contains a "charts" segment, even
when that directory is a legitimate deployment location under root. Narrow the
final return condition so it only skips Helm vendor/dependency charts inside an
actual deployment chart subtree, while still allowing real charts discovered
anywhere under root; keep the existing root/charts and dependency-directory
checks in should_skip_deployment_chart aligned with the discovery contract.
- Around line 78-85: The run_command helper currently lets subprocess.run raise
FileNotFoundError when helm or promtool is missing, which bypasses normal
validation handling. Update run_command to catch FileNotFoundError around the
subprocess.run call and return a failure-like result or otherwise surface a
clean error state that the callers in render_and_check.py can treat as a
validation failure. Keep the fix localized to run_command and preserve its
current contract for existing command execution paths.
In `@tests/scripts/test_promtool_integration.py`:
- Around line 15-19: The PromtoolIntegrationTest class is only gated on
PROMTOOL, but test_real_render_check_supports_non_deployments_path also depends
on Helm via render_and_check.py. Update the test class setup so the
Helm-dependent case is skipped unless both promtool and helm are available,
using the existing PromtoolIntegrationTest and
test_real_render_check_supports_non_deployments_path symbols to add the
additional Helm availability check before running that test.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 07702ffa-e7f8-453d-9513-d21baceb392c
📒 Files selected for processing (13)
.dockerignore.gitignoredocs/promtool-validation.mdsample/charts/mariadb-alerts/.helmignoresample/promtool-tests/mariadb-alerts/mariadb_alerts_test.yamlsample/promtool-tests/mariadb-alerts/rules/mariadb_alerts_rules.yamlscripts/extract_rules.pyscripts/promtool_test.shscripts/render_and_check.pytests/scripts/test_extract_rules.pytests/scripts/test_promtool_integration.pytests/scripts/test_promtool_test_sh.pytests/scripts/test_render_and_check.py
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 `@tests/scripts/test_render_and_check.py`:
- Around line 173-178: The test_run_command_reports_missing_binary_cleanly case
is environment-sensitive because it relies on a real missing command name; patch
subprocess.run in run_command’s test path to raise FileNotFoundError instead of
invoking missing-promtool, then keep the assertions on the returned code and
stderr message to verify the fallback behavior deterministically.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 8944e363-fa5a-4fe6-87c6-e1588cf786cb
📒 Files selected for processing (6)
sample/promtool-tests/mariadb-alerts/mariadb_alerts_test.yamlscripts/extract_rules.pyscripts/render_and_check.pytests/scripts/test_extract_rules.pytests/scripts/test_promtool_integration.pytests/scripts/test_render_and_check.py
🚧 Files skipped from review as they are similar to previous changes (3)
- tests/scripts/test_promtool_integration.py
- sample/promtool-tests/mariadb-alerts/mariadb_alerts_test.yaml
- scripts/extract_rules.py
rophy
left a comment
rophy
left a comment
There was a problem hiding this comment.
Clean, well-tested validation scripts. One minor note: main() in extract_rules.py re-inlines the extract + yaml.safe_dump logic that promtool_rules_yaml() already encapsulates — could call the helper to avoid drift. Not blocking.
Bump version from 1.3.0 to 1.4.0 to trigger a new image build and GitHub Release via CI. Since 1.3.0: deployment sync (#44), alert overview mode with column filters (#37), promtool validation (#41, #43), plus several fixes — enough new features to warrant a minor bump per this project's convention (1.2.0 -> 1.3.0 similarly bumped minor for the Gitea migration).
2 participants
Summary
Refs #40
Implements standalone/manual promtool validation scripts.
extract_rules.pyfor extractingPrometheusRule.spec.groupsfrom rendered Helm YAMLrender_and_check.pyfor chart/deploymenthelm template -> promtool check rulespromtool test ruleswrappersample/promtool-tests/deployments/pathsChart.lock/charts/*.tgzValidation
Passed:
python3 -m unittest discover -s tests/scriptsscripts/promtool_test.sh samplepython3 scripts/render_and_check.py chart --root samplepython3 scripts/render_and_check.py deployment --root sample --build-dependenciesnpm run buildnpm run lintKnown existing issue:
npm testhas 1 locale-sensitive failure unrelated to this change:tests/unit/git-lib.test.jsOn branch位於分支 masterRemaining
Summary by CodeRabbit