Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

#1 support multiple namespaces - we just need daemon to support it #2

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

KlavsKlavsen
Copy link

@KlavsKlavsen KlavsKlavsen commented Jul 22, 2020

Fixes #1 - This enables specifying multiple namespaces, so when running in local mode - it will simply add rolebinding to central SA - just need to know what to add to deployment, so daemon knows which namespaces to try and get metrics from (it currently just pulls from its own namespace).

@gkarthiks
Copy link
Owner

This is good.

But the actual codebase is not accepting multiple namespaces right. With the current codebase, i guess we need to deploy one pod per namespace.

But I like the idea of daemon sets. In that case, the codebase should be able to pull and scan across all the namespace. That needs to changed.

@KlavsKlavsen
Copy link
Author

Well - this would not entail running as a daemonset (if thats what you meant) - as those run one instance per physical node (which I see no use for here) - it merely needs the go code, to accept an array of "namespaces" - and then iterate over those and get pod resources - instead of only doing it for "current namespace" - or "clusterwide"..
This enables teams to use it, for taking responsibility for their OWN monitoring - and f.ex. having a prometheus (with a CRE per prometheus) - per project, and seperate monitoring out, so they belong to the project.. which is often the case, when developers are asked to do devops :)

We'd rather not run one instance of CRE per namespace (more pods to monitor, scrape etc.) and keep our "management" namespace for each project and have monitoring components live in there.

@gkarthiks
Copy link
Owner

Then it is good I believe. Can I go ahead and merge this?

@KlavsKlavsen
Copy link
Author

As I understand it - the GO code does not have any way, I can tell it which namespaces to "list pods resources" from ?
Once thats possible - I need to add the runScope.namespaces value to the container env for CRE - or as command argument or however you want it communicated to CRE process..
Once thats in place - its good to merge.

@KlavsKlavsen
Copy link
Author

I added useExistingRole support - so one can seperate role creation from the rest (needed in some RHEL openshift setups - where RBAC (but not rolebinding and SA creation) must be done by admin).

Did you have a look at wether or not it would be easy to add multiple namespace support to the Go code ?
I would propose to simply deliver namespaces as an env var like:
MONITOR_NAMESPACES="namespace1,namespace2,namespace3"

and then just "do what it does now" - in a loop over those - if that env var is set ?

If you agree with the approach, I'll gladly update Helm chart to expose that env var if namespaces is given as value.

@gkarthiks
Copy link
Owner

Hi @KlavsKlavsen apologies for the delayed response. I will change the Go code this weekend.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

where to specify specific namespaces to monitor?
2 participants