fix(fluent-bit-collector): only mount the data volume when storage.enabled#732
Open
yugstar wants to merge 2 commits into
Open
fix(fluent-bit-collector): only mount the data volume when storage.enabled#732yugstar wants to merge 2 commits into
yugstar wants to merge 2 commits into
Conversation
…abled
The DaemonSet guarded the `data` volume and volumeMount with
`{{- if .Values.storage }}`, but `.Values.storage` is always a non-empty
map, so the writeable hostPath `data` volume was mounted even when
`storage.enabled` is false (the default). On clusters that restrict
writeable hostPath access (restricted Pod Security), this could block
the pod from starting.
Guard both with `{{- if .Values.storage.enabled }}`, matching the
storage args guard already used elsewhere in the template.
Closes fluent#699
Signed-off-by: Aman Raj <aman.yug@gmail.com>
stevehipwell
requested changes
Jun 18, 2026
stevehipwell
left a comment
stevehipwell
left a comment
Collaborator
There was a problem hiding this comment.
Thanks for the PR @yugstar. Please could you undo the change to Chart.yaml and add an entry to the CHANGELOG under ## [UNRELEASED] as described in the contributing documentation.
…ntry Per review: revert the Chart.yaml version bump and the regenerated README version references, and record the fix under the CHANGELOG [UNRELEASED] section instead, as described in the contributing documentation. Signed-off-by: Aman Raj <aman.yug@gmail.com>
Author
|
Thanks for the review. I have reverted the Chart.yaml version bump along with the README version references it generated, and recorded the fix under the CHANGELOG [UNRELEASED] section instead. The only remaining change is the daemonset fix that gates the data volume on storage.enabled. |
Collaborator
|
@yugstar could you please rebase this PR and I'll try and get it merged before the next FB release. |
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
2 participants
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.
Description
The
fluent-bit-collectorDaemonSet guarded thedatavolume and volumeMount with{{- if .Values.storage }}. Since.Values.storageis always a non-empty map (it hasenabledandhostPathkeys), the condition was always true, so the writeable hostPathdatavolume was mounted even whenstorage.enabled: false(the default).On clusters that restrict writeable hostPath access (restricted Pod Security Standards / hardened setups), this could block the pod from starting. A commenter confirmed the issue is still present on
main.This changes both guards to
{{- if .Values.storage.enabled }}, matching the storage-args guard already used elsewhere in the same template.Verification
helm template(default,storage.enabled=false) → nodatavolume/volumeMount rendered (volumes areconfig,machine-id,logsonly).--set storage.enabled=true→datavolume present (unchanged behaviour when storage is enabled).helm lintpasses. Chart version bumped 1.0.7 → 1.0.8 and the README version references updated accordingly.Closes #699