-
Notifications
You must be signed in to change notification settings - Fork 5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[kube-prometheus-stack] Add condition to Alert Manager default dashboard to prevent adding the dashboard when AM is disabled #4707
base: main
Are you sure you want to change the base?
Conversation
My first thought is that there might be folks deploying this stack without alertmanager enabled but still wanting to get the dashboard for whatever reason. 🤔 |
I had a similar thought and debated implementing a new value similar to |
Fair enough. However this change needs to be done in a different way. please check the header of the file you changed. |
cde7c09
to
f8f6708
Compare
Updated to use the sync script |
f8f6708
to
237120d
Compare
I'd like to get a second opinion on this, the change might be considered a breaking change because something that might be in use is removed use once this is merged. @jkroepke @QuentinBisson pls 🙏 |
In general, I would favorite a more generic approach where individual dashboards can be toggled. |
…e dashboard when AM is disabled Signed-off-by: Phenix66 <[email protected]>
237120d
to
c7ed0be
Compare
Added an individual toggle identical to the one used for node exporter to do the same thing. |
Signed-off-by: Phenix66 <[email protected]>
abb7f83
to
b5983d0
Compare
What this PR does / why we need it
When AM is disabled, the default Grafana dashboard for AM is still added. Other services such as the Kube proxy, etcd, etc do not add their dashboards when disabled.
Which issue this PR fixes
(optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close that issue when PR gets merged)Special notes for your reviewer
Checklist
[prometheus-couchdb-exporter]
)