fluent / fluent-operator

Operate Fluent Bit and Fluentd in the Kubernetes way - Previously known as FluentBit Operator
Apache License 2.0
577 stars 246 forks source link

Fixes the parsing of disableComponentControllers in helm #1222

Closed mritunjaysharma394 closed 3 months ago

mritunjaysharma394 commented 3 months ago

What this PR does / why we need it:

This PR is needed to fix the parsing of disableComponentControllers. It seems like the value from helm is being parsed incorrectly. I added a small change in code locally and built a custom image to test with chart, while the manager binary itself worked fine without using chart but on parsing the value with chart, I got this logged:

2024-06-28T15:10:37Z    INFO    setup   Value of disabledControllers    {"value": "\"fluentd\""}
2024-06-28T15:10:37Z    ERROR   setup           {"error": "incorrect value for `-disable-component-controllers` and it will not be proceeded (possible values are: fluent-bit, fluentd)"}
main.main
        /workspace/main.go:122
runtime.main

Which is the reason why it is reporting it, it reads it as "\"fluentd\"".

It is happening because the printf function and quote function are unnecessary here because the --disable-component-controllers={{ . }} correctly appends the value without adding extra quotes. On making these changes locally, everything runs fine it seems related to it:

kubectl logs -n fluent pod/fluent-operator-7df5b4d96b-scphc            ✔  kind-kind ⎈ 
Defaulted container "fluent-operator" out of: fluent-operator, setenv (init)
2024-06-28T15:45:05Z    INFO    controller-runtime.metrics      Metrics server is starting to listen    {"addr": ":8080"}
2024-06-28T15:45:05Z    INFO    setup   starting manager
2024-06-28T15:45:05Z    INFO    Starting server {"path": "/metrics", "kind": "metrics", "addr": "[::]:8080"}
2024-06-28T15:45:05Z    INFO    Starting server {"kind": "health probe", "addr": "[::]:8081"}
2024-06-28T15:45:05Z    INFO    Starting EventSource    {"controller": "fluentbit", "controllerGroup": "fluentbit.fluent.io", "controllerKind": "FluentBit", "source": "kind source: *v1alpha2.FluentBit"}
2024-06-28T15:45:05Z    INFO    Starting EventSource    {"controller": "fluentbit", "controllerGroup": "fluentbit.fluent.io", "controllerKind": "FluentBit", "source": "kind source: *v1.ServiceAccount"}
2024-06-28T15:45:05Z    INFO    Starting EventSource    {"controller": "fluentbit", "controllerGroup": "fluentbit.fluent.io", "controllerKind": "FluentBit", "source": "kind source: *v1.DaemonSet"}
2024-06-28T15:45:05Z    INFO    Starting Controller     {"controller": "fluentbit", "controllerGroup": "fluentbit.fluent.io", "controllerKind": "FluentBit"}
2024-06-28T15:45:05Z    INFO    Starting EventSource    {"controller": "fluentbit", "controllerGroup": "fluentbit.fluent.io", "controllerKind": "FluentBit", "source": "kind source: *v1alpha2.FluentBit"}
2024-06-28T15:45:05Z    INFO    Starting EventSource    {"controller": "fluentd", "controllerGroup": "fluentd.fluent.io", "controllerKind": "Fluentd", "source": "kind source: *v1alpha1.Fluentd"}

Which issue(s) this PR fixes:

Fixes #1212

Does this PR introduced a user-facing change?

fix the parsing of disableComponentControllers in Helm Chart

Additional documentation, usage docs, etc.:

benjaminhuo commented 3 months ago

@mritunjaysharma394 Thanks! would you please sync this change to https://github.com/fluent/helm-charts/tree/main/charts/fluent-operator as well?

mritunjaysharma394 commented 3 months ago

Hi @benjaminhuo, thanks so much for the approval, I will definitely do that shortly, I was looking at the discussion in the issue and was confused so I hope that this change really fixes the issue? As some said it's a release issue but I think since you merged it, it somehow fixes or improves it?