feat(helm): package Grafana dashboards#2946
Open
osu wants to merge 3 commits into
Open
Conversation
Signed-off-by: Hasan Khan <hasank@nvidia.com>
Contributor
WalkthroughThe PR packages three Grafana dashboards into Helm, adds values, helpers, a conditional ConfigMap template, tests, and sidecar/setup documentation. It also adds one new core metric documentation row. ChangesGrafana Dashboard Helm Packaging
Core Metrics Documentation Update
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![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
🤖 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 `@helm/templates/_helpers.tpl`:
- Around line 11-13: The `nico.grafanaDashboardsName` helper is using only
`.Release.Name`, which can produce the same ConfigMap name for releases with the
same name in different namespaces and cause collisions. Update this helper to
incorporate the release namespace into the generated name, or add a
values-driven override for the ConfigMap name, and keep the result
truncated/trimmed so existing Helm upgrade behavior remains stable.
In `@helm/tests/fixtures/grafana-custom-values.yaml`:
- Around line 6-8: The Grafana dashboard label override is being rendered as a
null value instead of being removed, which breaks the expected manifest output.
Update the label handling in the Grafana values/template flow so the default
label is explicitly omitted or filtered out before rendering metadata.labels,
using the relevant label merge/helper path that processes grafana_dashboard and
sidecar.example.com/dashboard.
🪄 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: Enterprise
Run ID: 1572a944-6787-43ab-9555-219f1f1a5bbc
📒 Files selected for processing (10)
helm/PREREQUISITES.mdhelm/README.mdhelm/dashboards/nico-api-performance.jsonhelm/dashboards/nico-lifecycle.jsonhelm/dashboards/nico-overview.jsonhelm/templates/_helpers.tplhelm/templates/grafana-dashboards.yamlhelm/tests/fixtures/grafana-custom-values.yamlhelm/tests/grafana_dashboards_test.yamlhelm/values.yaml
Signed-off-by: Hasan Khan <hasank@nvidia.com>
Signed-off-by: Hasan Khan <hasank@nvidia.com>
🔍 Container Scan Summary
Per-CVE detail lives in the per-service |
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
1 participant
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.
Description
Package three production Grafana dashboards for NICo site overview, object-lifecycle diagnostics, and API performance. The Helm chart can optionally install them for a Grafana dashboard sidecar.
This change:
carbideandalt_metric_prefixmetric prefixes; andRelated issues
Closes #1696
Type of Change
Breaking Changes
The dashboard installer is disabled by default and does not install Grafana.
Testing
Passed locally:
jqstructure, unique UID, unique panel-ID, and field-config checks for every dashboardpromtool3.5.4: all 59 dashboard PromQL expressions parsed successfullyhelm lint helm --set global.image.repository=example.invalid/nico --set global.image.tag=testhelm lint helm -f helm/tests/fixtures/grafana-custom-values.yaml --set global.image.repository=example.invalid/nico --set global.image.tag=testhelm templaterenderskubectl apply --dry-run=client --validate=falseon the rendered ConfigMaphelm unittest --strict -f 'tests/grafana_dashboards_test.yaml' helm(4/4 pass)helm package helm: all three dashboard JSON files present in the chart archiveGitHub CI passed:
core-ci-passandrest-ci-pass.Additional Notes
The three JSON payloads total about 60 KiB. When enabled, the installer emits one release-scoped ConfigMap with the standard
grafana_dashboard: "1"sidecar label.The complete existing Helm unit suite was also run. It has the same nine stale nico-api certificate SAN expectation failures on this branch and pristine
origin/main: the tests do not yet include the recently addedcarbide-api.forgeSAN. This change does not touch those templates or tests.