feat: support yaml config file

cw-Guo commented 2 weeks ago

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #1008

Does this PR introduced a user-facing change?

1. fluent-bit supports yaml config mode
2. fluent-bit supports intput processors and output processors

Additional documentation, usage docs, etc.:

benjaminhuo commented 2 weeks ago

@cw-Guo Thanks, that's great to have yaml config files

cc @wanjunlei @wenchajun @Gentleelephant @adiforluls

cw-Guo commented 2 weeks ago

@benjaminhuo ~This is ready for review~. But the test cases are limited.

The idea behind this implementation can be found in the origin issue# 1008

To support processor, a map[string]interface{} is used to avoid defining the structure for the processors. Because the available processors in the official documents are very limited and are subjected to change.

We might add a new CRD for processors in the furture. (In practice, any filter can be a processor, without errors but this usage is not documented in the offical documents.)

A few LoadAsYaml functions haven't been implemented, I will try to implement them in the following few days.

[x] filter_types.go
[x] output_types.go

cw-Guo commented 2 weeks ago

To run it, a new image for fluent-bit need to be built.

adiforluls commented 2 weeks ago

I left a comment in the proposal about an edge case. And would like to see some test cases here if possible (unit tests at-least).

cw-Guo commented 2 weeks ago

I left a comment in the proposal about an edge case. And would like to see some test cases here if possible (unit tests at-least).

i will add more tests. For custom plugin, i will also try to incooperate it but i am afraid it will be hard.

adiforluls commented 2 weeks ago

I left a comment in the proposal about an edge case. And would like to see some test cases here if possible (unit tests at-least).

i will add more tests. For custom plugin, i will also try to incooperate it but i am afraid it will be hard.

Thoughts:

Old users who have custom plugin config will be on classic style by default, so no confusion here
A new user comes along, adds a custom plugin config in classic style and selects yaml style in the fbc CR. IMO this should result in operator throwing an error, if the user selects yaml style config, then they should provide yaml style configs in custom plugin and vice versa.

This is just my opinion, I'd let other maintainers pitch in as well on the PR.

cw-Guo commented 1 week ago

@adiforluls I can probably add one more field in the custom plugin to take in the unsupported plugins, just like the current processor implementation. https://github.com/fluent/fluent-operator/pull/1208/files#diff-487bd681e6fe01d15ade2e5a8650963714441e9282d9d6898c8c3ce1ab09aa43R76

cw-Guo commented 1 week ago

@benjaminhuo @adiforluls @wanjunlei @wenchajun @Gentleelephant May I have your reviews?

benjaminhuo commented 1 week ago

@benjaminhuo @adiforluls @wanjunlei @wenchajun @Gentleelephant May I have your reviews?

Sure, we'll review this. Thanks for the contribution. cc @wanjunlei @wenchajun @Gentleelephant

wanjunlei commented 6 days ago

I have some questions and suggestions.

This PR does not involve namespace CRD. Will this be implemented in the future?
In the long run, we don't need to maintain two sets of configuration files. We will remove the support for classic configuration files in future versions. So when the fluentbit version is greater than 2.0.0, can we consider using the YAML configuration file as the default configuration file?
It is not a good idea to splice yaml files. I suggest defining a structure to generate yaml config file.

Just some suggestions, not blocking this pr.

cw-Guo commented 6 days ago

@wanjunlei ❤️ Thanks for the review and suggestions. I have fixed the import orders.

This PR does not involve namespace CRD. Will this be implemented in the future?

I am not too sure about how the namespace CRDs work. I didn't use it in my previous experience. But I did add support for:

Filter,
Output

They are passed in to fluentbitconfig_controller.go. From my understanding, this will be enough but please correct me if i understand it wronly.

In the long run, we don't need to maintain two sets of configuration files... So when the fluentbit version is greater than 2.0.0, can we consider using the YAML configuration file as the default configuration file?

It is not a good idea to splice yaml files. I suggest defining a structure to generate yaml config file.

I agree that we don't want to maintain two sets of configuration files and I am also against splicing yaml files. But I also hasitate to make the (hard) decision for the users. Avoiding breaking changes is the start point of my design. I am afraid only supporting yaml config file format would possibly make some users abandon the project.

If the community has decided to only support the yaml config file format, I am happy to see that a refractor for fluent-operator or a newer api version.

wanjunlei commented 6 days ago

@wanjunlei ❤️ Thanks for the review and suggestions. I have fixed the import orders.

This PR does not involve namespace CRD. Will this be implemented in the future?

I am not too sure about how the namespace CRDs work. I didn't use it in my previous experience. But I did add support for:

Filter,

Output

They are passed in to fluentbitconfig_controller.go. From my understanding, this will be enough but please correct me if i understand it wronly.

In the long run, we don't need to maintain two sets of configuration files... So when the fluentbit version is greater than 2.0.0, can we consider using the YAML configuration file as the default configuration file?

It is not a good idea to splice yaml files. I suggest defining a structure to generate yaml config file.

I agree that we don't want to maintain two sets of configuration files and I am also against splicing yaml files. But I also hasitate to make the (hard) decision for the users. Avoiding breaking changes is the start point of my design. I am afraid only supporting yaml config file format would possibly make some users abandon the project.

If the community has decided to only support the yaml config file format, I am happy to see that a refractor for fluent-operator or a newer api version.

OK, got it. The namespace CRD just needs to change the FIlter and Output.

benjaminhuo commented 5 days ago

I agree that we don't want to maintain two sets of configuration files and I am also against splicing yaml files. But I also hasitate to make the (hard) decision for the users. Avoiding breaking changes is the start point of my design. I am afraid only supporting yaml config file format would possibly make some users abandon the project.

If the community has decided to only support the yaml config file format

The old config file format might exist for some time for the yaml format to mature and catch up. There are quite a lot of things to catch up.

benjaminhuo commented 5 days ago

Thanks @cw-Guo very much for such a big enhancement! I've invited you as fluent-operator maintainer. Looking forward to more contributions for the yaml format support! @patrick-stephens @agup006 would you help to invite @cw-Guo to the fluent org?

fluent / fluent-operator