aws-samples / amazon-cloudwatch-container-insights

CloudWatch Agent Dockerfile and K8s YAML templates for CloudWatch Container Insights.
MIT No Attribution
163 stars 107 forks source link

update fluent bit optimized configuration with use_kubelet option ena… #83

Closed DrewZhang13 closed 2 years ago

DrewZhang13 commented 2 years ago

Issue #, if available: https://github.com/fluent/fluent-bit/pull/3025 Fluent Bit is supporting use_kubelet opetion for EKS to address large cluster scalability issue in fluent bit version 1.7.2. https://fluentbit.io/announcements/v1.7.2/ And this feature is supported in aws-for-fluent-bit in version 2.12.0.

Description of changes: The change is about enable the use_kubelet option in fluent bit filter config and change related configuration in template to support this feature.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

PettitWesley commented 2 years ago

@DrewZhang13 I know from your docs that this setting requires extra setup: https://docs.fluentbit.io/manual/pipeline/filters/kubernetes#optional-feature-using-kubelet-to-get-metadata

I see from the changes here that all of the required extra permissions/settings are configured. It's important that all customers can use the Container Insights config and can deploy it without issue. I just want to double check before we finally merge this that we checked with EKS team that its safe to add these settings for most/common/beginner customers?

DrewZhang13 commented 2 years ago

Hi Wesley, i worked with PA Nithish for implementation and testing on this feature. And got confirmed from ESK PM Mike for enabling this feature when submitting this CR. I think it should be safe.

PettitWesley commented 2 years ago

@DrewZhang13 Thanks for confirming. I will deploy this on a new EKS cluster tomorrow just to be safe.

PettitWesley commented 2 years ago

Tested it on a newish cluster, and it works as expected, no issues:

$ kubectl get nodes
NAME                                       STATUS   ROLES    AGE   VERSION
ip-10-0-1-174.us-west-2.compute.internal   Ready    <none>   97d   v1.21.5-eks-9017834
PettitWesley commented 2 years ago

@SaxyPandaBear Please take a look at this one as well

SaxyPandaBear commented 2 years ago

Ah I merged it a bit too hastily. I just now saw that this is supported in 2.12 and I think without #95 we're still pinned to 2.10

PettitWesley commented 2 years ago

@SaxyPandaBear Ack, I'll fix #95 ASAP/right now.

SaxyPandaBear commented 2 years ago

Much appreciated. I'm good to approve that once it gets updated. I'll keep an eye out for it