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

Remove duplicate Grafana dashboards #377

Merged
merged 2 commits into from
Jan 23, 2020

Conversation

branden
Copy link
Contributor

@branden branden commented Jan 23, 2020

This PR fixes an error that causes the prometheus-operator chart to generate configmaps with conflicting names.

This removes dashboards that are duplicated by these dashboards introduced with #309:

Testing

I tested this by deploying a Konvoy cluster with the prometheus addon enabled, then deployed another cluster with the prometheus addon disabled, and deployed this modified chart to the second cluster with the addon's chart values. I then compared the affected dashboards on both clusters and observed that they looked the same.

These dashboards are duplicated by dashboards under
`templates/grafana/dashboards-1.14/`.
@branden branden added the ready ready label Jan 23, 2020
@branden branden requested review from gracedo, alejandroEsc and a team January 23, 2020 02:20
@branden branden self-assigned this Jan 23, 2020
@GoelDeepak
Copy link
Contributor

Nice troubleshooting @branden :)

Copy link
Contributor

@gracedo gracedo left a comment

Choose a reason for hiding this comment

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

🙌

@branden
Copy link
Contributor Author

branden commented Jan 23, 2020

Looks like CI is failing on the kommander chart, for some reason:

[18:32:05]	[Step 1/2] Linting chart with values file 'staging/prometheus-operator/ci/01-provision-crds-values.yaml'...
[18:32:05]	[Step 1/2] 
[18:32:05]	[Step 1/2] >>> helm lint staging/prometheus-operator --values staging/prometheus-operator/ci/01-provision-crds-values.yaml
[18:32:05]	[Step 1/2] ==> Linting staging/prometheus-operator
[18:32:05]	[Step 1/2] Lint OK
[18:32:05]	[Step 1/2] 
[18:32:05]	[Step 1/2] 1 chart(s) linted, no failures
[18:32:05]	[Step 1/2] 
[18:32:05]	[Step 1/2] Linting chart with values file 'staging/prometheus-operator/ci/02-test-without-crds-values.yaml'...
[18:32:05]	[Step 1/2] 
[18:32:05]	[Step 1/2] >>> helm lint staging/prometheus-operator --values staging/prometheus-operator/ci/02-test-without-crds-values.yaml
[18:32:05]	[Step 1/2] ==> Linting staging/prometheus-operator
[18:32:05]	[Step 1/2] Lint OK
[18:32:05]	[Step 1/2] 
[18:32:05]	[Step 1/2] 1 chart(s) linted, no failures
[18:32:05]	[Step 1/2] Error linting charts: Error processing charts
[18:32:05]	[Step 1/2] ------------------------------------------------------------------------------------------------------------------------
[18:32:05]	[Step 1/2]  ✖︎ kommander => (version: "0.3.17", path: "stable/kommander") > Chart version not ok. Needs a version bump!
[18:32:05]	[Step 1/2]  ✔︎ prometheus-operator => (version: "8.3.8", path: "staging/prometheus-operator")
[18:32:05]	[Step 1/2] ------------------------------------------------------------------------------------------------------------------------
[18:32:05]	[Step 1/2] make: *** [ct.lint] Error 1
[18:32:05]	[Step 1/2] Makefile:105: recipe for target 'ct.lint' failed
[18:32:05]	[Step 1/2] Process exited with code 2
[18:32:05]	[Step 1/2] Process exited with code 2
[18:32:05]	[Step 1/2] Step Chart Lint (Command Line) failed

make ct.lint passes when I run it locally against this branch.

@joejulian
Copy link
Contributor

Odd. The kommander chart wasn't altered. I'll merge this one.

@joejulian joejulian merged commit b6d2fe8 into mesosphere:master Jan 23, 2020
mesosphere-teamcity pushed a commit that referenced this pull request Jan 23, 2020
…rds These dashboards are duplicated by dashboards under . * Bump prometheus-operator chart version
@branden branden deleted the dedupe-dashboards branch January 23, 2020 03:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready ready
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants