Save data to database

[zugao]

Summary

• Data from the clusters and exoscale provider is matched, filtered, refined and ultimately saved to the database
• Changed naming in other packages
• Added unit tests
• Added docs

Local Testing:

1. Configure K8S_SERVER_URL and K8S_TOKEN environment variables for k8s cluster.
2. Configure an unrestricted Exoscale IAM Key and save into env variables -> EXOSCALE_API_KEY and EXOSCALE_API_SECRET
3. Create a postgres database local environment from here and save the url with credentials into env variable -> ACR_DB_URL
4. Make sure to create some exoscale buckets in the cluster which is linked to the correct Exocale organisation (where the IAM Key was created point 1) before running the application.
5. Build and run the program -> exoscale-metrics-collector objectstorage --log-level 1 --cluster-config clusters.yaml

Checklist

• Categorize the PR by setting a good title and adding one of the labels:
  bug, enhancement, documentation, change, breaking, dependency
  as they show up in the changelog
• Update tests.

[davidgubler]

Since you clearly like the cli parser library (which I don't), you should replace all the os.GetEnv() calls by this library. Currently there is a weird mix. Unfortunately because Github doesn't let me comment on collapsed lines in the DIFF (I can view them but not add a comment, thanks Github) I cannot directly point out the affected lines...

[davidgubler]

Since you're not prefixing the environment variables (which I like, because I think prefixing environment variables is pointless and annoying), you should remove this code from the main.go file.

`// env combines envPrefix with given suffix delimited by underscore.
func env(suffix string) string {
return envPrefix + suffix
}

// envVars combines envPrefix with each given suffix delimited by underscore.
func envVars(suffixes ...string) []string {
arr := make([]string, len(suffixes))
for i := range suffixes {
arr[i] = env(suffixes[i])
}
return arr
}`

[davidgubler]

Another thing I don't like is the fact that the kubernetes config is coming from a file (kubernetes.go line 26). I see no reason why the yaml shouldn't be put into an environment variable. Using environment variables for some things and files for other things just makes things unnecessarily complex and clumsy to use (e.g. if I need to switch between multiple environments).

I also don't like the approach of having a single yaml document for configuring multiple clusters. This means that when setting this up (e.g. creating secrets) there must be some code that understands yaml and can merge all the different configurations. This is just unnecessarily complicated. I would much prefer one (or multiple, as it was before) environment variables per configured cluster to make things easy to work with. (you can still have yaml inside this one environment variable if you feel that this is easier than having e.g. 2 environment variables per cluster as my original implementation had)

[ccremer]

Another thing I don't like is the fact that the kubernetes config is coming from a file (kubernetes.go line 26). I see no reason why the yaml shouldn't be put into an environment variable. Using environment variables for some things and files for other things just makes things unnecessarily complex and clumsy to use (e.g. if I need to switch between multiple environments).

I also don't like the approach of having a single yaml document for configuring multiple clusters.

Here's another idea: work directly with kubeconfig files

• The parsing of this config is directly integrated into the kubernetes client that we're using. We'd require no code, just setting a KUBECONFIG var.
• If choosing a different environment, either maintain multiple kubeconfig files and switch with KUBECONFIG env var, or maintain multiple cluster definitions in a single kubeconfig file and switch environment using kubectl config use-context $context. (In the latter approach, I'm not quite sure if we can initialize a client in code with a certain context or if it's only using the current context)
• It may make sense to run the metrics collector per cluster individually (as separate processes/containers/pods) and not all clusters in one execution.
• The kubeconfig, if not generated, copied or adjusted from elsewhere, can be generated using various kubectl commands, e.g. kubectl config set-cluster and kubectl config set-credentials or with oc login --token ... --server ...

[zugao]

Since you clearly like the cli parser library (which I don't), you should replace all the os.GetEnv() calls by this library. Currently there is a weird mix. Unfortunately because Github doesn't let me comment on collapsed lines in the DIFF (I can view them but not add a comment, thanks Github) I cannot directly point out the affected lines...

I had a wrong reason to parse them but after playing around a bit more with the cli library I got it right and now the parsing is is done by this library.

Another thing I don't like is the fact that the kubernetes config is coming from a file (kubernetes.go line 26). I see no reason why the yaml shouldn't be put into an environment variable. Using environment variables for some things and files for other things just makes things unnecessarily complex and clumsy to use (e.g. if I need to switch between multiple environments).

The only reason I decided to go with the file is because how big the text may get. I don't feel it's the right away to put a potential huge text file into an env variable.

I also don't like the approach of having a single yaml document for configuring multiple clusters. This means that when setting this up (e.g. creating secrets) there must be some code that understands yaml and can merge all the different configurations. This is just unnecessarily complicated. I would much prefer one (or multiple, as it was before) environment variables per configured cluster to make things easy to work with. (you can still have yaml inside this one environment variable if you feel that this is easier than having e.g. 2 environment variables per cluster as my original implementation had)

I really wanted to avoid 2 env variables per cluster and the next (less bad) thing for me was to put that data neatly into a file. From my part I don't see having difficulties to create a secret out of this file (maybe I'm missing something?), such approach is being widely used like prometheus or alert manager. In any case I can understand your concern and I'm open for better suggestions. Also the parsing of yaml file is pretty straight forward in go and doesn't make the code much more difficult then it is with parsing an n number of env variables.

@ccremer Regarding your suggestion for the cluster configs, it seems sensible but we should discuss this in a short call so that we are all aligned before I commit to change the code.

[zugao]

During our BF we decided to remove multiple cluster support and instead run one job per cluster to populate the database. With this change it was natural to just use the same pattern aka environment variables like we did for other systems (exoscale and database).

[ccremer]

Another thing I don't like is the fact that the kubernetes config is coming from a file (kubernetes.go line 26). I see no reason why the yaml shouldn't be put into an environment variable. Using environment variables for some things and files for other things just makes things unnecessarily complex and clumsy to use (e.g. if I need to switch between multiple environments).

Just to recap the latest discussion: We opted to go for 2 environment variables (token + url). But each execution of the binary is targeted towards 1 cluster. So if you need to switch environment (another cluster), you need to reset the env vars.
That also means for production, we deploy a CronJob for each target cluster.