Skip to content

Conversation

aslafy-z
Copy link

@aslafy-z aslafy-z commented Oct 3, 2025

This PR includes several changes to allow the use of this Helm chart with existing infrastructure components such as Prometheus Operator and Grafana.

  • global.alerts.enabled (new value): enable/disable deployment of alerts
  • global.alerts.kind (new value): alerts container resource (configmap or prometheusrule) (Prometheus Operator compatibility)
  • global.dashboards.enabled (new value): enable/disable deployment of dashboards
  • global.dashboards.labels (new value): allow to set dashboard configmaps a label (Grafana Sidecar compatibility)
  • runbookViewer.enabled (new value): enable/disable deployment of the runbook viewer
  • Fallback to GitHub-hosted runbook when runbookViewer is disabled
  • Only deploy collector configmap when grafana-agent is enabled
  • Prefix dashboards Grafana names by Coder
  • Prefix dashboards configmap names and Grafana uid (posponed Make Grafana dashboards' UIDs consistent #63) by coder-

It does not fix #31 but helps people that only want to deploy dashboards and alerts for example.

This PR was NOT tested in real world conditions.

@aslafy-z aslafy-z force-pushed the feat/dashboard-labels branch 3 times, most recently from 8acd058 to 4cd9c53 Compare October 3, 2025 14:45
@aslafy-z aslafy-z marked this pull request as ready for review October 3, 2025 14:52
@aslafy-z aslafy-z requested a review from dannykopping as a code owner October 3, 2025 14:52
@aslafy-z aslafy-z force-pushed the feat/dashboard-labels branch 3 times, most recently from 47bb49f to dd9620a Compare October 3, 2025 14:58
@dannykopping
Copy link
Collaborator

@aslafy-z thanks very much for your contribution. I'll be on PTO until Mon 13th, and my colleague who is co-maintaining this with me is away as well. Would it be ok with you if I got to this when I return?

Signed-off-by: Zadkiel AHARONIAN <hello@zadkiel.fr>
@aslafy-z aslafy-z force-pushed the feat/dashboard-labels branch from dd9620a to 651a327 Compare October 3, 2025 15:04
@aslafy-z
Copy link
Author

aslafy-z commented Oct 3, 2025

@dannykopping sure, enjoy your pto!

Copy link
Collaborator

@dannykopping dannykopping left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall LGTM! Thanks for a great contribution @aslafy-z
Left a few notes

@@ -0,0 +1,34 @@
{{- if and .Values.global.alerts.enabled (eq .Values.global.alerts.format "prometheusrule") -}}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you tested this out with Prometheus disabled and observed everything working as expected?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a mention to the description. I had no time to test these changes for the moment.
image

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They don't necessarily have to have been tested in production, but the code must have at least been validated manually using Prom Operator for us to accept the change.

configMap: metrics-alerts
configMap: coder-metrics-alerts
readonly: true
optional: true
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the other configmaps also be optional?

aslafy-z and others added 2 commits October 13, 2025 11:50
Co-authored-by: Danny Kopping <dannykopping@gmail.com>
aslafy-z and others added 2 commits October 13, 2025 12:05
Signed-off-by: Danny Kopping <danny@coder.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Ability to use dashboards outside of the helm chart

2 participants