grafana-operator icon indicating copy to clipboard operation
grafana-operator copied to clipboard

Support grafanadashboards in a configmap without having to define a grafanadashboard CR

Open SerialVelocity opened this issue 3 years ago • 6 comments

This is the same as the closed issue #249. I could not re-open it and could not get a response from @pb82 about why the issue was closed.

Is your feature request related to a problem? Please describe. Dashboards created by helm charts tend to have a grafana_dashboard flag set on them and are usually imported using kiwigrid/k8s-sidecar. These are not automatically imported by grafana-operator

(If applicable)If your feature request solves a bug please provide a link to the community issue

Describe the solution you'd like For the config maps to be read and imported.

Describe alternatives you've considered Using cronjobs to convert them in the background. It's hacky.

Additional context N/A

Existing solutions github.com/kiwigrid/k8s-sidecar

SerialVelocity avatar Jul 31 '21 17:07 SerialVelocity

@SerialVelocity we do not support any helm chart for the grafana-operator, bitnami have published a helm chart https://github.com/bitnami/charts/pull/3287 and if you want any changes to happen to it you will need to create a issue/PR with them.

Or are we misunderstanding you? If so please can you elaborate more on what you think is missing?

NissesSenap avatar Aug 01 '21 07:08 NissesSenap

Hey @NissesSenap, this is not about grafana-operator being a Helm chart. This is to do with many helm charts creating their dashboards by having a grafana_dashboard label associated with their dashboard config map. This also sometimes applies to non-helm dashboards too.

Sorry, the original issue had a comment with a link to some dashboards but it failed to show an example of what the config maps actually look like.

Here's an example I found using search: https://github.com/datastax/pulsar-helm-chart/blob/299864641317f295efbf0d8037204e13f438e268/helm-chart-sources/pulsar/templates/monitoring/grafana-dashboards.yaml#L33

SerialVelocity avatar Aug 01 '21 14:08 SerialVelocity

If i understand you correctly you want to be able to import a grafan dashboard that is in a configmap, is this correct?

That is already supported: https://github.com/grafana-operator/grafana-operator/blob/master/deploy/examples/dashboards/DashboardFromConfigMap.yaml

All you need to do is to create a grafana dashboard and point on the cm. You just have to make sure that the grafana dashboard got the correct labels so the operator can find it.

I might misunderstand you again, if that is the case please share some pure yaml and i guess try to explain it in another way.

NissesSenap avatar Aug 01 '21 20:08 NissesSenap

You are right, I want to be able to import them, but I want to import them without having to create grafana dashboard CRD for each.

So, when I go to upgrade a helm chart that I have, if they add new grafana dashboards, they appear automatically in Grafana.

SerialVelocity avatar Aug 01 '21 21:08 SerialVelocity

Okay, since this actually have nothing to do with helm I changed the name since this applies to all yaml, it dosen't have anything to do with a template engine.

We could tell the operator to monitor configmaps with a specific label just like we do with the grafanadashboards. Since we already support CM usage together with the grafanadashboard CR my guess is that feature won't be anything super prioritized from the maintainers point of view.

Would you be willing to add this feature your self @SerialVelocity?

NissesSenap avatar Aug 02 '21 06:08 NissesSenap

We could tell the operator to monitor configmaps with a specific label just like we do with the grafanadashboards. Since we already support CM usage together with the grafanadashboard CR my guess is that feature won't be anything super prioritized from the maintainers point of view.

Yes, either directly, or via the grafanadashboard CR extension which allows you to specify a label instead of namespace/name

Would you be willing to add this feature your self @SerialVelocity?

I'm not able to currently and not sure when I'll be able to. If I find some free time, I'll have a go at it.

(also cc @tlipatov-asapp since you mentioned you work around this issue too)

SerialVelocity avatar Aug 02 '21 08:08 SerialVelocity

@NissesSenap @pb82 @weisdd Should we consider this for v5?

Imho, with the more opinionated approach from v5+ we should stay away from non-CR installation methods, however I think a workaround might exist by being able to mount a configmap to the same directory as our defaults in ConfigMapsMountDir or GrafanaProvisionDashboardVolumeName (however, we don't use i in code right now, which won't solve this feature request)

It would also mean we can avoid dealing with configmaps and label selectors etc. and just let a user define a configmap and we automatically attach it to an instance if the configmap exists when specified in the grafana CR

hubeadmin avatar Apr 13 '23 20:04 hubeadmin

I agree with you @HubertStefanski i don't think we should support this. But if users want to mount a cm that contain dashboards and make grafana read that dashboard using built in features of grafana i see a problem with that.

But isn't this supported already? My understanding from reading the documentation you can just add some cm as volumes and your grafana instance should detect them. And this all be possible to do in v5 today, https://grafana.com/docs/grafana/latest/administration/provisioning/#dashboards

https://grafana.com/docs/grafana/latest/administration/provisioning/#provision-folders-structure-from-filesystem-to-grafana

I'm unsure if i think we should document this since i feel this is an anti pattern. We could write a small blog about it though. Not that it makes it less official.

NissesSenap avatar Apr 14 '23 05:04 NissesSenap

I guess my ask here was for the operator to support generic config maps just by them having a label on them. I don't particularly want to go through each helm chart/yaml file, check whether they have added new dashboards, add it to grafana to mount it, etc. I would prefer for the operator to automatically pick up the existing ones based on labels that are in use today.

As in, I would prefer for this operator to be generic, rather than having to either write a webhook to auto-create your special CRDS, or manually creating your CRDs and checking each release manually.

It seems like a needed feature to have greater support for what is out there. Going through all charts and then mounting anything they create as volumes is not really feasible (and then, what's the point in having this operator if they have to be mounted directly into grafana?).

Hope that makes sense :)

SerialVelocity avatar Apr 18 '23 17:04 SerialVelocity

We have decided that we won't support this in v5. We have created a crd for a reason. Implementing this feature would generate lots of code and would also make rbac managment harder.

We would be willing to implement the same support that exist in v4 to v5 where it's possible to call on a configmap located in a crd. This have been done in https://github.com/grafana-operator/grafana-operator/pull/999, we won't merge this pr due to its size. But i have spoken to him and it's okay for anyone to pick things from the PR as you want and break out to a new pr.

Sorry for the bad news @SerialVelocity .

NissesSenap avatar May 11 '23 04:05 NissesSenap

In my opinion, we should have a CRD that enables us to use the config map to import this to grafana because its a kind of default in dashboard distribution just have the label on config map , in my case, I just enabled metamonitoring in Mimir chart to figure out these dashboards cant be imported to grafana operated out of the box

https://github.com/grafana/mimir/blob/main/operations/helm/charts/mimir-distributed/templates/metamonitoring/grafana-dashboards.yaml

rafilkmp3 avatar May 30 '23 21:05 rafilkmp3

same here .. the kyverno chart just creates a dashboard configmap: https://github.com/kyverno/kyverno/blob/main/charts/kyverno/templates/grafana/dashboard.yaml but with the grafana-operator it is not possible to import this configmap out-of-the-box .. sad :/

jkleinlercher avatar Aug 08 '23 10:08 jkleinlercher