[Bugfix][Helm] skip lmcache dashboard when cache server is disabled#987
[Bugfix][Helm] skip lmcache dashboard when cache server is disabled#987bitnik wants to merge 1 commit into
Conversation
Signed-off-by: Kenan Erdogan <kenanerdogan@gmail.com>
There was a problem hiding this comment.
Code Review
This pull request introduces conditional rendering for the lmcache-dashboard.json Grafana dashboard, ensuring it is only deployed when cacheserverSpec.enabled is set to true. It also adds and updates Helm unit tests to validate this behavior. The reviewer provided valuable feedback pointing out a potential nil pointer evaluation error if cacheserverSpec is omitted or null in the values, suggesting a safer traversal using the dig function.
| {{- if .Values.grafanaDashboards.enabled }} | ||
| {{- range $path, $bytes := .Files.Glob "dashboards/*.json" }} | ||
| {{- $filename := base $path }} | ||
| {{- if and (eq $filename "lmcache-dashboard.json") (not $.Values.cacheserverSpec.enabled) }} |
There was a problem hiding this comment.
If a user overrides .Values and completely omits cacheserverSpec or sets it to null, accessing $.Values.cacheserverSpec.enabled will cause a template rendering error (nil pointer evaluation). To make this check robust and prevent potential rendering failures, we can use the Sprig dig function to safely traverse the nested map with a default fallback value.
{{- if and (eq $filename "lmcache-dashboard.json") (not (dig "cacheserverSpec" "enabled" false $.Values)) }}
-swhen doinggit commit[Bugfix],[Feat], and[CI].When
grafanaDashboards.enabledis true, the chart renders aConfigMapfor every JSON file indashboards/*.json, includinglmcache-dashboard.json. This happened regardless of whether the cache server was actually deployed, so clusters running withcacheserverSpec.enabled: falseended up with an LMCache Grafana dashboard for a component that isn't running and it showes panels with no-data.This PR makes the dashboard rendering conditional:
lmcache-dashboard.jsonis now skipped whencacheserverSpec.enabledisfalse. All other dashboards continue to render as before.