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

Fixes the parsing of disableComponentControllers in helm #1222

Conversation

mritunjaysharma394
Copy link
Contributor

@mritunjaysharma394 mritunjaysharma394 commented Jun 28, 2024

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 benjaminhuo merged commit 5fea345 into fluent:master Jul 2, 2024
2 checks passed
@benjaminhuo
Copy link
Member

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

@mritunjaysharma394
Copy link
Contributor Author

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?

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.

bug: Arg --disable-component-controllers still not working properly
2 participants