Skip to content
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] Optional Windows-specific dashboards and rules #3083

Merged
merged 16 commits into from
Jul 3, 2023

Conversation

lippertmarkus
Copy link
Contributor

@lippertmarkus lippertmarkus commented Mar 1, 2023

What this PR does / why we need it

Which issue this PR fixes

This PR allows enabling Windows-specific dashboards and rules from kubernetes-mixin based on the kube-prometheus Windows addon. Windows nodes are widely used in Kubernetes clusters, and monitoring them should be as easy as for Linux nodes.

Special notes for your reviewer

As being an addon the dashboards and rules aren't part of the kube-prometheus /manifests folder which is currently used by the hack scripts.

Therefore, I generated the manifests with the addon enabled once, temporarily used those adapted manifests with the hack scripts and added the resulting new files to the PR, all gated behind windowsMonitoring.enabled.

The rules and dashboards depend on the deployment of windows_exporter like described here: https://github.com/prometheus-community/windows_exporter/blob/master/kubernetes/kubernetes.md

Checklist

  • DCO signed
  • Chart Version bumped
  • Title of the PR starts with chart name (e.g. [prometheus-couchdb-exporter])

Signed-off-by: Markus Lippert <[email protected]>
Signed-off-by: Markus Lippert <[email protected]>
@monotek monotek removed their request for review March 6, 2023 13:39
@lippertmarkus
Copy link
Contributor Author

I don't know why it automatically added so many reviewers automatically, sorry for that

charts/kube-prometheus-stack/values.yaml Show resolved Hide resolved
charts/kube-prometheus-stack/values.yaml Outdated Show resolved Hide resolved
@lippertmarkus lippertmarkus requested review from monotek and removed request for zanhsieh, desaintmartin, rsotnychenko, dotdc, mrueg and tariq1890 March 10, 2023 14:22
@lippertmarkus
Copy link
Contributor Author

@andrewgkew @gianrubio @gkarthiks @scottrigby @Xtigyro can one of you please give us a second feedback on this?

Signed-off-by: Markus Lippert <[email protected]>
Signed-off-by: Markus Lippert <[email protected]>
@lippertmarkus
Copy link
Contributor Author

@jkroepke sorry was busy the last weeks. The windows-exporter chart works fine for me, thanks! I added it to the PR.

@jkroepke jkroepke removed their request for review June 29, 2023 10:21
Signed-off-by: Markus Lippert <[email protected]>
Signed-off-by: Markus Lippert <[email protected]>
@lippertmarkus
Copy link
Contributor Author

@monotek could you please review again?

@lippertmarkus
Copy link
Contributor Author

@monotek and merge please :)

@monotek monotek merged commit 990fcbc into prometheus-community:main Jul 3, 2023
@lippertmarkus
Copy link
Contributor Author

Matiasmct pushed a commit to giffgaff/prometheus-charts-backup that referenced this pull request Aug 25, 2023
prometheus-community#3083)

* feat: add windows addon dashboards and rules

Signed-off-by: Markus Lippert <[email protected]>

* merge upstream

Signed-off-by: Markus Lippert <[email protected]>

* add headers

Signed-off-by: Markus Lippert <[email protected]>

* fix version

Signed-off-by: Markus Lippert <[email protected]>

* update header and add nl

Signed-off-by: Markus Lippert <[email protected]>

* add windows-export chart

Signed-off-by: Markus Lippert <[email protected]>

* spell error

Signed-off-by: Markus Lippert <[email protected]>

* add NL

Signed-off-by: Markus Lippert <[email protected]>

---------

Signed-off-by: Markus Lippert <[email protected]>
Signed-off-by: Markus Lippert <[email protected]>
Co-authored-by: MH <[email protected]>
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.

7 participants