Add hot_reload.timeout and hot_reload.ensure_thread_safety support to FluentBitConfig#1926
Open
devodev wants to merge 1 commit intofluent:masterfrom
Open
Conversation
… FluentBitConfig These params are missing from the FluentBitConfig and ClusterFluentBitConfig specs which prevents configuring hot reload. This is important because by default, hot reload timeout is not set and defaults to unbounded, which can cause inifinite wait on output tasks that don't cooperate. Also split Service.Params into multiple functions to fix gocyclo lint: cyclomatic complexity 32 of func `(*Service).Params` is high (> 30) Signed-off-by: Alexandre Barone <abalexandrebarone@gmail.com>
devodev
force-pushed
the
pr/devodev/fluent-bit-config-hot-reload-timeout
branch
from
3f225c1 to
1c2ee3d
Compare
There was a problem hiding this comment.
Pull request overview
Adds missing Fluent Bit hot reload configuration knobs to the FluentBitConfig/ClusterFluentBitConfig Service spec so users can bound hot reload wait time and optionally enforce thread-safety during reloads (aligning with issue #1925).
Changes:
- Extend the shared
ServiceAPI type withhotReloadTimeoutandhotReloadEnsureThreadSafetyand render them into generated Fluent Bit config. - Update unit tests to assert both classic and YAML config outputs include the new keys when set.
- Regenerate CRDs/manifests/Helm CRDs and update API docs to expose the new fields.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| apis/fluentbit/v1alpha2/clusterfluentbitconfig_types.go | Adds the two new Service fields and emits Hot_Reload.Timeout / Hot_Reload.Ensure_Thread_Safety in rendered params. |
| apis/fluentbit/v1alpha2/clusterfluentbitconfig_types_test.go | Extends expected rendered configs (classic + YAML) and test config to cover the new settings. |
| apis/fluentbit/v1alpha2/zz_generated.deepcopy.go | Regenerates deepcopy logic for the new Service fields. |
| config/crd/bases/fluentbit.fluent.io_fluentbitconfigs.yaml | Regenerated CRD schema exposing the new Service fields (namespaced). |
| config/crd/bases/fluentbit.fluent.io_clusterfluentbitconfigs.yaml | Regenerated CRD schema exposing the new Service fields (cluster). |
| charts/fluent-operator/crds/fluentbit.fluent.io_fluentbitconfigs.yaml | Regenerated Helm CRD for FluentBitConfig including the new fields. |
| charts/fluent-operator/crds/fluentbit.fluent.io_clusterfluentbitconfigs.yaml | Regenerated Helm CRD for ClusterFluentBitConfig including the new fields. |
| charts/fluent-operator-crds/templates/fluent-bit/fluentbit.fluent.io_fluentbitconfigs.yaml | Regenerated templated Helm CRD including the new fields. |
| charts/fluent-operator-crds/templates/fluent-bit/fluentbit.fluent.io_clusterfluentbitconfigs.yaml | Regenerated templated Helm CRD including the new fields. |
| manifests/setup/fluent-operator-crd.yaml | Regenerated install manifest CRDs including the new fields. |
| manifests/setup/setup.yaml | Regenerated setup manifest including the new fields. |
| docs/fluentbit.md | Regenerated API docs to document the new Service fields. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
These params are missing from the FluentBitConfig and ClusterFluentBitConfig specs which prevents configuring hot reload.
What this PR does / why we need it:
Add
hot_reload.timeoutandhot_reload.ensure_thread_safetysupport to FluentBitConfig and ClusterFluentBitConfig.This is important because by default, hot reload timeout is not set and defaults to unbounded, which can cause inifinite wait on output tasks that don't cooperate.
Which issue(s) this PR fixes:
Fixes: #1925
Does this PR introduced a user-facing change?