mhausenblas / krs

A command line tool for capturing and serializing Kubernetes resource statistics in OpenMetrics format
https://mhausenblas.info/krs/
Apache License 2.0
95 stars 8 forks source link

Handling more resources #3

Closed nicolasbernard closed 6 years ago

nicolasbernard commented 6 years ago

Hi,

First, thanks for krs :) .

kubectl get all doesn't get all resources, and krs doesn't count all of them anyway. Can we change that?

My proposal: Step 1: replace kubectl get allby kubectl get deploy,po,svc. Step 2: make toOpenMetrics() more naive by blindy use the Kind value and build a dynamic namespaceStats{}. Step 3: make the "deploy,po,svc" part of step 1 overridable at runtime with a flag. (Like /app/krs --get 'svc,po,deploy,ing,pdb' $NAMESPACE)

Step 4 for extra points: make another flag for "all we can have", which build an exhaustive list of kinds to get by parsing the return of kubectl api-resources --verbs=list.

I can make the patch if you approve.

Thanks. --Nicolas

mhausenblas commented 6 years ago

@nicolasbernard thanks a lot for the feedback. Yes, I agree with your observation and here's what I'm thinking: initially (just have a look at the history) I was using kubectl get events, so the most generic way one could think of. Turns out that kubectl get events is, erm, suboptimal since it apparently only gives you updates about certain resources such as pods. So, I was looking for another kind of catchall, hence ended up with kubectl get all.

Now, I like your proposal and I'm inclined to go with it, I'm just not sure if at the current point in time I'm ready to take on PRs, I think for now I'd rather implement it myself. Don't get me wrong, I find your offer to patch that up very generous, it's just it's too early in the krs lifecycle (barely an MVP ;) that I feel comfortable with it.

So, what I'm gonna do is essentially take your design proposal and implement the following:

$ krs --resources="svc,po,deploy,ing,pdb" $NAMESPACE

With --resources defaulting to po,svc,deploy to make it backwards compatible.

As an aside, get all captures quite a bit (as you can see from the RBAC permissions) and indeed the whole thing was always planned with this extension possibility in mind, IOW, the idea was to simply add resource kinds in openmetrics.go as we go ;)

Again, thanks for your feedback, much appreciated, and if you're still interested to contribute once the code base has settle a bit, please do let me know!

nicolasbernard commented 6 years ago

I was the "earlyness" of the project that make me volunteering the patch, not sure is the project was a toy or a "real sideproject", so no problems :) .

I think adding resource kinds as we go is a limited approch, I could have been more explicit about it, my dynamic way was drived by the possibility of capturing CRDs.

mhausenblas commented 6 years ago

Oh sorry, I should have been more explicit @nicolasbernard … I'm a fan of DDD (demand driven development). That is, if something is useful for folks, the status moves from toy/side project to real/serious thing. I think with the current trajectory we're seeing this one moving to serious. Also, in terms of code base, I have no issues at all throwing away everything and start from scratch. For example, rather than depending on kubectl which is a pain, I'd love to go for client-go, then, taking all your feedback into account, all the feedback I got from other places, not yet in GitHub issues. Let's take one step at a time. I implement the core you suggested now, if there's more interest, more people using it, maybe you wanna chime in as a co-author? :)

mhausenblas commented 6 years ago

FYI @nicolasbernard I've now released v0.2 which addresses some of your asks above.