feat(new transform): Inital kubernetes_pod_metadata transform

[ktff]

Closes #1072
Ref #1867

Implemetation notes:

• Supported fields:
  • name
  • namespace
  • creation_timestamp
  • deletion_timestamp
  • labels
  • annotations
  • node_name
  • hostname
  • priority
  • priority_class_name
  • service_account_name
  • subdomain
  • host_ip
  • ip
• Fields are added under log schema kubernetes_key. Which has default value kubernetes.
• Tests and example config are using RBAC authorization to allow Vector access to API server. RBAC is usualy enabled by default, but this requirement should be documented .

Open questions

• Is default value kubernetes best for log schema kubernetes_key ?

• Is this a breaking change because log schema has been changed ? (Edit: Will not be with fix(config): Partial LogSchema #1923)

• How to deal with Event interpreting label/annotation keys like "app.kubernetes.io/name" as nested?

Todo

• After this is merged, open an issue to update kubernetes_source naming scheme to use kubernetes_key. This will be a breaking change.

• Add delete timeout.

• Add jitter.

[binarylogic]

@ktff I added #1867 as a closing issue here. Please remove if that is not the case, but it looks like it at least partially resolves it?

[ktff]

@binarylogic

but it looks like it at least partially resolves it?

Yes, this two:

• Add a log_schema.kubernetes_key option
• Use this key in the kubernetes_metadata transform

[binarylogic]

@ktff would you mind adding docs?

Also, do we have a setting that controls the refresh rate against the k8s API? I ask because I have spoken to users that have experienced issues with this with fluent*. For example, very large clusters (5000+ pods) effectively DDOS'd the k8s API. A configuration option should allow users to reduce the refresh rate.

ref fluent/fluent-bit#1399

[ktff]

@binarylogic good question. We are currently only proactively asking for metadata once and then passively waiting for api server to send us changes, so we don't have refresh rate, but we have something similar.

We are using watch endpoint to listen for metadata changes for pods on local node, so there are basically two states:

1. Connecting to api server.
   Exposing the retry_timeout as configuration option would enable users to decrease the pressure on api server in large clusters. Also, because 1. is problematic when large number of Vectors try to connect at the same time, deploying Vector with this transform on large clusters at once is problematic.
   To mitigate that, two more changes would help:

   • Make timeout duration probabilistic. Uniform distribution on range [0,2*retry_timeout] would have largest deviation, a median of retry_timeout, and also be indirectly depended on the size of the cluster. This would help spread out connection retries through time which would spread our pressure on api server more evenly.
   • Add the timeout even when trying to connect for the first time.

2. Waiting for changes from api server.
   Shouldn't be problematic because it's either self regulating or an issue that needs to be solved with extra feature when it arises.

It sounds reasonable to me to add this as it's a small change with great benefits.

@LucioFranco what do you think?

[LucioFranco]

Looking good I left a few comments inline.

[LucioFranco]

So I have two main questions:

1. How does using the watch client affect us, this means that we will need to have a tcp connection open to the api server for the duration of the vector instance. I don't think this should be a large issue but I have seen other metadata transforms provide the option to use the watch or a poll based api.

2. I think the biggest problem with large scale deployments would be vector trying to hit the api server all at once. We should introduce some sort of jitter for the initial request and subsequent requests so we can avoid the stampeding herd problem.

I think this follow closely to what you said @ktff, what do you think?

Note: we may want to add jitter to our retry policy as well so that our other sinks can benefit from the same.

https://aws.amazon.com/builders-library/timeouts-retries-and-backoff-with-jitter/?did=ba_card&trk=ba_card

[ktff]

@LucioFranco

Yep, we are thinking the same thing. I'll add the jitter. And we could open an issue for adding it to the retry policy, and possibly extract this implementation.

I suspect poll is useful for usecases when the used portion of metadata won't be changing, so only fetching it once makes sense. Other than that, I think we would need to wait and see. I also didn't found any Fluent issue regarding watch vs poll.

[LucioFranco]

@binarylogic do you have anymore info you cold provide us about the issues you were describing?

[alexgavrisco]

This PR adds everything we need tn order to go with Vector in EKS! Thanks a ton!
I'm going to test today Vector 0.8.x for a small cluster, however for production use we really need the ability to filter by pod_name/namespace etc.
Is there a tiny chance this feature will be released with 0.9? No pressure, I just want to set correct expectations on my end.

[ktff]

@Alexx-G Great!

If you are using Kubernetes v1.14 or higher, the kubernetes source will output pod_name and pod_namespace. Although those names will be changed with the coming 0.9 version which will most likely have this transform in it.

[alexgavrisco]

@ktff Thanks for update!
Yesterday I tested 0.8.x on an EKS cluster, and technically there's enough metainfo (contaienr_name, pod_name and namespace) so we can settle down with Vector. I tested it in a relatively small cluster, once it passes our validation phase, we'll see how it behaves for bigger clusters (100-200 worker nodes).
Since this feature makes API calls to kubernetes, the kubernetes deployment must have corresponding clusterrole bindings, right?

[binarylogic]

Thanks for all of your help @Alexx-G

Since this feature makes API calls to kubernetes, the kubernetes deployment must have corresponding clusterrole bindings, right?

@ktff if that is the case, woudl you mind updating https://github.com/timberio/vector/issues/1450 to include details on that. I'm working on the k8s docs now.

[alexgavrisco]

@binarylogic Thank you for your effort! I'd gladly offer more help.
Today I'll whitelist more services that Vector will collect logs from and I'll see how it goes. But from my previous tests I don't expect any issues there.
Once I finish integrating Vector, I'll see if I can contribute with some feedback to #1397 (since it should contain all resources vector needs to work properly in a Kubernetes cluster).

[ktff]

@Alexx-G

Since this feature makes API calls to kubernetes, the kubernetes deployment must have corresponding clusterrole bindings, right?`

This transform yes, and its clusterrole binding is in this PRs yaml.

@binarylogic

if that is the case, woudl you mind updating #1450 to include details on that. I'm working on the k8s docs now.

clusterrole bindings are a part of vector-daemonset.yaml, and besides having RBAC authorization enabled, they don't require other user intervention. It is documented in the yaml, with other such necessities, so just mentioning yaml as the goto for more in depth information should be enough.

RBAC authorization is documented in this transforms documentation as it's the one using it. source can do without it.

[binarylogic]

Thank you. That makes sense.

[ktff]

@binarylogic could you review the docs, and also I'm not sure how to fix the error in
make generate / ci/circleci: check-blog

[binarylogic]

Yep. I'm on it today. Once I finish the docs this is good to merge, correct?

[binarylogic]

Docs are done! Looks really good.

[binarylogic]

I am making a few more documentation changes and will merge afterwards.

[LucioFranco]

@ktff can we add a test around verifying labels are inserted correctly and all other values are inserted as we expect?

[binarylogic]

Nice work @ktff and @LucioFranco.

[marcbachmann]

Did this get removed from master because of performance issues?

[jszwedko]

Hi @marcbachmann . This was removed since it is implemented by the kubernetes_logs source. We have discussed bringing it back in the future though, to separate the reading of logs and annotation of metadata.