Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged

Conversation

DrewZhang13
Copy link
Contributor

Issue #, if available:
fluent/fluent-bit#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
Copy link
Contributor

@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
Copy link
Contributor Author

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
Copy link
Contributor

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

@PettitWesley
Copy link
Contributor

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
Copy link
Contributor

@SaxyPandaBear Please take a look at this one as well

Copy link
Contributor

@SaxyPandaBear SaxyPandaBear left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@SaxyPandaBear SaxyPandaBear merged commit 45055ae into aws-samples:master May 12, 2022
@SaxyPandaBear
Copy link
Contributor

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
Copy link
Contributor

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

@SaxyPandaBear
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants