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

feat: add namespace selector using env variable

Open atos-cosmin-gavagiuc opened this issue 1 year ago • 2 comments

add namespace selector 1433

atos-cosmin-gavagiuc avatar Feb 20 '24 13:02 atos-cosmin-gavagiuc

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar Feb 20 '24 13:02 CLAassistant

@NissesSenap @weisdd @pb82 @theSuess thoughts? IMHO doesn't harm to have another way to specify this

hubeadmin avatar Feb 21 '24 15:02 hubeadmin

Sounds reasonable to me. Before merging, it would be nice if we can add this feature to the helm chart https://github.com/grafana/grafana-operator/blob/04b8181d5ea2ccb4299fa934f1150cc127e0a5f5/deploy/helm/grafana-operator/values.yaml#L11

How do we see this working with RBAC, is it up to the grafana-operator admins to fix this on their own? I'm fine with that, but it would be nice to document.

It would also be nice if we added an e2e test for this. It's probably something that @eddycharly could help with. It would be similar to https://github.com/grafana/grafana-operator/pull/1437, but I'm unsure how we would make the operator be installed in different modes in the best way. One for all the existing tests and this one using the new namespace label selector.

NissesSenap avatar Feb 24 '24 14:02 NissesSenap

@NissesSenap i'm lacking context here, what is this supposed to achieve ?

eddycharly avatar Feb 24 '24 20:02 eddycharly

@eddycharly running the operator in the new mode called namespace_selector. Using a label on the namespace, and depending on that applying dashboard etc to grafana instances.

I guess we need to run another instance of the operator. Create 2 namespaces, one with the label and one without and see what only one of the dashboards gets picked up by the grafana instances. Or something like that.

Writing on my phone before going to bed, so sorry if I didn't think everything through.

NissesSenap avatar Feb 24 '24 21:02 NissesSenap

@atos-cosmin-gavagiuc , can you look at the comments in https://github.com/grafana/grafana-operator/pull/1434#issuecomment-1962391130? It won't get merged until some kind of documentation exist.

NissesSenap avatar Mar 12 '24 09:03 NissesSenap

@atos-cosmin-gavagiuc , can you look at the comments in #1434 (comment)? It won't get merged until some kind of documentation exist.

@NissesSenap, I've updated the helm chart 6699ee2. I'm not familiar with helm and I might have done it wrong.

atos-cosmin-gavagiuc avatar Mar 12 '24 11:03 atos-cosmin-gavagiuc

I update the helm config so it works with a string like environment: dev

We will keep RBAC in helm to cluster level access, since we can't force which namespace will need the access, since it's based on labels.

Please write some docs about this new feature here: https://github.com/grafana/grafana-operator/blob/master/docs/docs/grafana.md#where-should-the-operator-look-for-grafana-resources and we should be able to merge.

Thanks for your work @atos-cosmin-gavagiuc .

NissesSenap avatar Mar 12 '24 20:03 NissesSenap

@NissesSenap I've added the a short description in the docs 510a9b7f6fd742f6f685489deab8f1e0d517ce25

atos-cosmin-gavagiuc avatar Mar 14 '24 14:03 atos-cosmin-gavagiuc

@NissesSenap I've fixed that trailing whitespace.

atos-cosmin-gavagiuc avatar Mar 14 '24 16:03 atos-cosmin-gavagiuc