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

[fluentd] Add separate aggregator chart #67

Open
stevehipwell opened this issue Jan 28, 2021 · 12 comments
Open

[fluentd] Add separate aggregator chart #67

stevehipwell opened this issue Jan 28, 2021 · 12 comments
Labels
enhancement New feature or request

Comments

@stevehipwell
Copy link
Collaborator

The two use cases for Fluentd are very different in implementation and as such rather than trying to shoehorn both into a single chart it would make sense to have a separate fluentd-aggregator chart. I already have a fluentd-aggregator chart in my repo and would be happy to create a PR to add it to this repo and deprecate mine.

It would also be useful to have an aggregator Docker image to handle the common aggregation endpoints; an aggregator isn't as concerned with size and if the plugins are layered on top of the base image it would keep the layers easily portable. I've created my own image for my Helm chart, although the chart will work fine with the base Fluentd image (by default it outputs to null and has stdout debugging available), but an official version would be better.

I'd like to call out @mrd2 for his work on PR #55 for the Fluentd collector chart, this would be intended to augment this not replace it.

@naseemkullah
Copy link
Collaborator

Thanks @stevehipwell , would it be totally unfeasible to shoehorn it into the one @mrd2 worked on with a switch like mode: aengt|aggregator ? I understand a whole lot of logic would need to be added but is it still possible?

@stevehipwell
Copy link
Collaborator Author

@naseemkullah I'd personally avoid that approach (one of the reasons why I wrote a chart rather than using an existing well known Fluentd chart) due to additional complexity and cognitive load required. I prefer the two chart approach for something like this.

@naseemkullah
Copy link
Collaborator

I understand, but N.B. this has been done successfully for fluent bit :

# kind -- DaemonSet or Deployment
kind: DaemonSet

@stevehipwell
Copy link
Collaborator Author

I know it's an option but does anyone use it and is it a "good" approach? If you want an example of what it could look like for Fluentd take a look at the bitnami chart.

@dioguerra

This comment has been minimized.

@stevehipwell
Copy link
Collaborator Author

@mrd2 I'm offering to submit a PR for this.

@stevehipwell
Copy link
Collaborator Author

I'm still keen to explore this, specifically a separate Fluentd aggregator chart and image optimised for aggregating logs from Fluent Bit and distributing them.

@naseemkullah
Copy link
Collaborator

cc @edsiper

@dioguerra
Copy link
Collaborator

@stevehipwell are you still working on this?

@dioguerra dioguerra added the enhancement New feature or request label Aug 26, 2021
@stevehipwell
Copy link
Collaborator Author

@dioguerra yes, I have a chart and corresponding Docker image that are currently used on a number of production clusters. I'd like to hand both over if at all possible.

@dioguerra
Copy link
Collaborator

dioguerra commented Aug 26, 2021

Do you think we can squeeze this into fluentd?

It would be nice to make a Aggregator/Standalone switch. Does it make sense? How different is the aggregating functionality from edditing the respective fields?
I mean, at this point i'm asking, what can we add that it is not available in the bitnami offered fluentd...
https://github.com/bitnami/charts/tree/master/bitnami/fluentd

I will have a look when i get time. If this works as i am expecting i want to make a push and add it to magnum, but all in it's due time :)

@stevehipwell
Copy link
Collaborator Author

@dioguerra I guess the first question is if there is the appetite to create and maintain a Fluentd aggregator image in the Fluent group (unlike with Fluent Bit this is required)? If there is then it shouldn't be too hard to combine the charts if that's what's wanted. I'd also suggest aligning the fluent-bit & fluentd charts so they follow a common pattern; so probably a breaking change in both cases.

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

No branches or pull requests

3 participants