-
Notifications
You must be signed in to change notification settings - Fork 69
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
WIP: Adding dashboard directory for grafana dashboards and script to combine the dashboards #432
base: master
Are you sure you want to change the base?
Conversation
…fic json file. Adding create and delete configmap function to combine prombench.json and node-metrics.json into an single configmap Signed-off-by: Raj Babu Das <[email protected]> Signed-off-by: Raj Babu Das <[email protected]>
Signed-off-by: Raj Babu Das <[email protected]>
cc @geekodour. dashboard's configmap is successfully created in this way. I check the text diff with the previous config map. It looks the same(except the JSON format changes). But after applying all resources, the dashboards are not showing in the grafana. I will fix this problem later. Let me know if this approach is good or not. |
cc0ccc8
to
232cd69
Compare
Signed-off-by: Raj Babu Das <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR!
It'll be good to keep the provider code limited to auth provider and k8s code. Having to do with anything dashboards and grafana should not be in the scope of the provider code.
I think this change is also hardcoding filenames and names of the configmaps which has quite a few issues on itself :( It would be better if we maintain the flexibility to choose filenames and other parameters.
In #420 I simply meant generating grafana_dashboard_dashboards_noparse.yaml
file from grafana dashboard exports(json files), Ideally would be done by script or in some generic way to deal with generating one configmap from multiple normal files. Nothing to do with the provider.
That way rest of our code will remain the same and nothing changes :) What I think this PR is trying to do is specifically apply grafana dashboard files, the infra
cli is less concered about what the content of the ConfigMap
is (in our case grafana dashboard files) :)
@@ -138,3 +144,75 @@ func MergeDeploymentVars(ms ...map[string]string) map[string]string { | |||
} | |||
return res | |||
} | |||
|
|||
func getK8sClientSet() (*kubernetes.Clientset, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We already have the clientset at pkg/provider/k8s/k8s.go
:)
Thanks @geekodour for reviewing this PR. I will come up with a better solution using bash script. |
Signed-off-by: Raj Babu Das [email protected]