fix(collector): render ClusterFluentBitConfig referenced by Collectors - #2028
Open
l1ghtman2k wants to merge 2 commits into
Open
fix(collector): render ClusterFluentBitConfig referenced by Collectors#2028l1ghtman2k wants to merge 2 commits into
l1ghtman2k wants to merge 2 commits into
Conversation
The Collector controller waits for a Secret named after spec.fluentBitConfigName in the Collector's own namespace before it creates anything. Nothing ever rendered that Secret: the FluentBitConfig controller only iterated FluentBitList, so a ClusterFluentBitConfig that was only referenced by a Collector was skipped. The result was a Collector that silently requeued forever with no StatefulSet, Service, ServiceAccount or config Secret, and an empty status. Extend the FluentBitConfig controller to also iterate CollectorList and render the referenced ClusterFluentBitConfig into the Collector's namespace, and watch Collectors so the config is rendered as soon as one is created. The plugin rendering body is factored out into listClusterPlugins/renderAndStoreConfig and shared by both code paths; the FluentBit path is unchanged, including the spec.namespace override and namespaced (multi-tenant) FluentBitConfig handling. Configs already rendered for a FluentBit in this loop are never re-rendered for a Collector, so a Collector cannot drop the namespaced plugins of a DaemonSet sharing the same config. Collectors have no namespaced FluentBitConfig selector, so only cluster scoped plugins are rendered for them. Also make the Collector controller observable while it is waiting: log which Secret is missing instead of failing silently, requeue with a bounded interval instead of a hot loop, and watch Secrets so the StatefulSet is created as soon as the configuration shows up. Fixes fluent#1436 Signed-off-by: Aibek Zhylkaidarov <aibek.zhylkaidarov@hpe.com>
l1ghtman2k
force-pushed
the
fix/collector-fluentbitconfig-rendering
branch
from
August 12, 2026 19:30
91c8b25 to
c9c5fdc
Compare
l1ghtman2k
marked this pull request as draft
August 12, 2026 19:45
l1ghtman2k
marked this pull request as ready for review
August 12, 2026 21:39
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a functional gap in the operator where Collector resources referencing a ClusterFluentBitConfig would never get their configuration rendered (and the Collector controller would silently wait forever), by extending the config rendering controller to also reconcile Collector-referenced configs and by making Collector reconciliation observable/reactive to config Secret creation.
Changes:
- Refactors
FluentBitConfigReconcilerto renderClusterFluentBitConfigSecrets for bothFluentBitandCollectorreferences, with explicit precedence rules to avoid Collector overwriting DaemonSet configs. - Improves
CollectorReconcilerbehavior when the config Secret is missing (logged + bounded requeue) and adds a Secret watch to wake Collectors immediately once config is rendered. - Adds controller tests covering Collector config rendering, YAML format handling, namespace pinning behavior, and precedence/notification behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
controllers/fluentbitconfig_controller.go |
Lists ClusterFluentBitConfig once per reconcile and renders config Secrets for both FluentBit and Collector consumers. |
controllers/collector_controller.go |
Logs + bounds requeue when config Secret is missing and watches Secrets to trigger immediate Collector reconciliation on config availability. |
controllers/collector_config_test.go |
Adds unit tests validating Collector-related config rendering and guarding FluentBit rendering behavior after refactor. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The Collector Secret watch had no predicate, so collectorsForSecret ran for every Secret event in the cluster. It correctly enqueued nothing for unrelated Secrets, but still allocated and discarded a CollectorList each time (~16us, ~17KB, 57 allocs per event on a busy cluster). Filter the watch on the fluent.io/config-hash annotation that FluentBitConfigReconciler stamps onto every config Secret it renders, and promote that key to a shared constant so the contract between the two controllers is named rather than duplicated as a literal. A Secret predating the annotation is filtered out, but the config reconciler treats a missing annotation as a change and rewrites it, so the resulting event carries the annotation and passes. Signed-off-by: Aibek Zhylkaidarov <aibek.zhylkaidarov@hpe.com>
Contributor
Author
|
Addressed the copilot review, left the comment |
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.
Fixes #1436
Problem
Create a
Collectorreferencing aClusterFluentBitConfigviaspec.fluentBitConfigNameand the operator creates nothing — no StatefulSet, Service, ServiceAccount or Secret — and logs no error..statusstays{}.Two interacting sites cause it:
collector_controller.gorequires a Secret namedspec.fluentBitConfigNameand silently requeues when it is absent:fluentbitconfig_controller.gois the only thing that renders that Secret, and it only iteratesFluentBitList:grep -c CollectorList controllers/*.goreturns 0 on bothv3.9.0andmaster, so no released version can render configuration for a Collector.Fix
Reconcilenow listsClusterFluentBitConfigListonce (previously re-listed per FluentBit) and drives two paths:reconcileFluentBits— existing behaviour, unchangedreconcileCollectors— renders configs referenced byCollectorCRs into the Collector's namespace, which is wherecollector_controller.golooksRendering is shared, not duplicated (
listClusterPlugins+renderAndStoreConfig). AddsWatches(&Collector{})so creating a Collector triggers a reconcile, plus the matching RBAC marker (make manifests generateproduced no diff).Also replaces the silent requeue in
collector_controller.gowith a logged, bounded one and a Secret watch — that silence is what made this hard to diagnose.Precedence and edge cases
NamespacedFluentBitCfgSelector) would strip the DaemonSet's multi-tenant filters and the two loops would fight, flipping the config hash. Covered by a test.cfg.Spec.Namespaceis honoured for consistency with the FluentBit path, but if it pins a namespace other than the Collector's the Collector still waits — that case now logs a warning. A maintainer may prefer the Collector namespace to always win; happy to change it.CollectorSpechas noNamespacedFluentBitCfgSelector. Existing CRD limitation, not introduced here.Testing
Six new tests in
controllers/collector_config_test.go. Reverting onlyfluentbitconfig_controller.goto master reproduces the bug:TestFluentBitNamespacedConfigStillRenderedguards the refactor: the renderedfluent-bit.conf(tail input, generatedrewrite_tag, namespaced tenant output) is byte-identical before and after.The only failure, on master and this branch alike, is
tests/e2e/fluentd, which needs a live cluster and is excluded frommake test.Verified on real clusters
minikube, same manifests, only the operator image swapped:
3.9.0Config stays operator-managed: adding a
ClusterFilterafterwards re-rendered the Secret (config-hashchanged) and the running pods picked it up with zero restarts.A 27-node cluster, 3-replica Collector: the config Secret rendered (impossible on stock 3.9.0), the StatefulSet reached 3/3 with three PVCs bound, and it has been ingesting steadily since.
One deployment note for users, not a code issue: the operator creates the Collector's ClusterRole, so
spec.rbacRulesmust be a subset of what the operator's ServiceAccount already holds — otherwise Kubernetes privilege-escalation prevention rejects it (attempting to grant RBAC permissions not currently held) and the StatefulSet is never created.