Cleaned up some of the logic in main

[ghost]

By introducing a function which gets the collectors interface, I wanted to know if there would ever be a case when there would be more than one collector?

IE:

NewClusterCollector
NewHelmV2Collector
NewHelmV3Collector

or is it always just one of these?

[ghost]

Additionally, I wanted to know what action to take when inputs are empty? @stepanstipl Should we throw or...?

[stepanstipl]

if there would ever be a case when there would be more than one collector?

@david-doit-intl yes, multiple collectors at once should be allowed, ie. typical scenario is that workloads in the cluster are deployed both directly (Cluster collector) and via Helm, and we want to scan all of those.

what action to take when inputs are empty?

Empty inputs are fine too, we print how many resources we have retrieved from individual collectors if there is none - no issue.

TL;DR: The ides of default use case is: Just run kubent, without any additional config, and it tries to detect as many problematic resources as we can.

[stepanstipl]

I think that moving the logic to individual functions, away from the main "spaghetti" is a nice improvement. 👍

But I would prefer to keep the actual retrieval of the resources collector.Get() to happen after all the collectors are initialized. Rationale: resource retrieval can take a while on large clusters/slow connections, and it's good for the user to fail fast & early if some preconditions are not met (like the collector failed to init because of bad kubeconfig, or the file for file collector does not exist).

Perhaps moving just the collection logic/loop to a separate function, smth. like processCollectors(collectors []collector.Collector) []interface{} (please come up with better name 😄 )?

[ghost]

I think that moving the logic to individual functions, away from the main "spaghetti" is a nice improvement.

But I would prefer to keep the actual retrieval of the resources collector.Get() to happen after all the collectors are initialized. Rationale: resource retrieval can take a while on large clusters/slow connections, and it's good for the user to fail fast & early if some preconditions are not met (like the collector failed to init because of bad kubeconfig, or the file for file collector does not exist).

Perhaps moving just the collection logic/loop to a separate function, smth. like processCollectors(collectors []collector.Collector) []interface{} (please come up with better name )?

Yes, we can do a defer as well, but ok will change!

[ghost]

@stepanstipl maybe also the collection.Get(), should be async then, if that is the case? https://gobyexample.com/goroutines

So we can run the get or the resource retrival all at once if they exist.

[ghost]

defer -> https://tour.golang.org/flowcontrol/12,

something like this so the Get collection can happen after!

[stepanstipl]

defer

Not sure how that would look like - .Get() would still have to be taken out of storeCollector, I think... Typically I've seen defer used more for closing connections and cleaning up.

maybe also the collection.Get(), should be async then

Hm, that is a good point and would say yes, but :)... because most of the collectors talk to the cluster, I'm not sure if hitting it with 3 connections, all at once, wouldn't degrade the resulting performance/cause issues with the network. But definitely a good point to think about... maybe worth running some tests....

[ghost]

let me show you the defer first and then we can have a look at async stuff if needed

[ghost]

@stepanstipl have updated this code :)

[stepanstipl]

@david-doit-intl I think it looks good 🚀 Do you plan to add tests for the new funcs? (I'm happy to merge as it is)

[ghost]

@stepanstipl adding tests now :)