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.

Sign up for GitHub

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

Jump to bottom

Support file system as storage layer in service section of fluenbit #825

Merged
merged 1 commit into from
Jul 10, 2023
Merged

Support file system as storage layer in service section of fluenbit #825

merged 1 commit into from
Jul 10, 2023

Conversation

karan56625
Copy link
Collaborator

@karan56625 karan56625 commented Jul 7, 2023 •
edited
Loading

What this PR does / why we need it:

  • Add Support file system as storage layer in service section of fluenbit.
  • Add Support to use file system as storage for Input

Which issue(s) this PR fixes:

Fixes #826

Does this PR introduced a user-facing change?

Additional documentation, usage docs, etc.:

@benjaminhuo
Copy link
Member

@karan56625 Thanks for the pr.
You'll need to resolve the conflicts by rebasing the upstream and then sign your commit:

git commit -s --amend
git push -f

@karan56625 karan56625 changed the title Support system layer in service section of fluenbit Support storage layer in service section of fluenbit Jul 7, 2023
adiforluls
adiforluls reviewed Jul 7, 2023
View reviewed changes
charts/fluent-operator/templates/fluentbit-clusterinput-tail.yaml Outdated
@@ -25,6 +25,8 @@ spec:
skipLongLines: {{ .Values.fluentbit.input.tail.skipLongLines }}
db: /fluent-bit/tail/pos.db
dbSync: Normal
storageType: {{ .Values.fluentbit.input.tail.storageType }}
pauseOnChunksOverlimit: {{ .Values.fluentbit.input.tail.pauseOnChunksOverlimit }}
Copy link
Member

@adiforluls adiforluls Jul 7, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pauseOnChunksOverlimit field is only functional when the storage type is fileSystem right? If that's the case then maybe add a check for this field to see what the storage type is

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although it will by default off and also no effect if it is enabled with fileSystem as storage. Still I have added check on this. Its always better to have one check on higher level.

@karan56625 karan56625 force-pushed the karak/support_file_system_as_storage branch from 2712a73 to 0cb2240 Compare July 7, 2023 14:53
@karan56625 karan56625 changed the title Support storage layer in service section of fluenbit Support file system as storage layer in service section of fluenbit Jul 7, 2023
@karan56625
Copy link
Collaborator Author

@karan56625 Thanks for the pr. You'll need to resolve the conflicts by rebasing the upstream and then sign your commit:

git commit -s --amend
git push -f

fixed

@wenchajun
Copy link
Member

This is the case for statefulset only? Does it output logs in pvc?

@karan56625
Copy link
Collaborator Author

karan56625 commented Jul 8, 2023 •
edited
Loading

This is the case for statefulset only? Does it output logs in pvc?

This is to configure the storage type of buffer that is getting used by fluent-bit pipeline. By default, buffer storage is memory. In this PR, we allow customer to customise that in fluent-bit operator to use file-system as buffer storage type instead of memory only.

see: https://docs.fluentbit.io/manual/administration/buffering-and-storage

@wenchajun
Copy link
Member

1688821201385
Do we need to add the same to input and output?

wenchajun
wenchajun previously approved these changes Jul 8, 2023
View reviewed changes
benjaminhuo
benjaminhuo reviewed Jul 9, 2023
View reviewed changes
apis/fluentbit/v1alpha2/clusterfluentbitconfig_types.go Outdated
@@ -80,6 +101,8 @@ type Service struct {
LogLevel string `json:"logLevel,omitempty"`
// Optional 'parsers' config file (can be multiple)
ParsersFile string `json:"parsersFile,omitempty"`
// Configure a global environment for the storage layer in Service
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@karan56625 I think we should add a note here to tell users it's better to set up volume and volumemount separately for this storage, it should be hostpath type for fluentbit daemonset. we shouldn't write chuncks directly to containers. And the volume & volumemount can be added like https://github.com/fluent/fluent-operator/blob/master/charts/fluent-operator/values.yaml#L129

We'd better add file buffer config to https://github.com/fluent/fluent-operator/blob/master/charts/fluent-operator/values.yaml#L129 too

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@benjaminhuo , I have added a note in that comment. Also one example is put in the values.yaml. Let me know if any change is required in those.

@karan56625
Copy link
Collaborator Author

1688821201385
Do we need to add the same to input and output?

that is taken care of already.

benjaminhuo
benjaminhuo reviewed Jul 10, 2023
View reviewed changes
charts/fluent-operator/values.yaml Outdated
# Uncomment the code if you intend to create the volume for buffer storage in case the storage type "filesystem" is being used in the configuration of the fluentbit service.
# - name: hostBuffer
# hostPath:
# path: /tmp/
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe like below?

Suggested change
# path: /tmp/
# path: /tmp/fluent-bit-buffer

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed.

@karan56625
Support system layer in service section of fluenbit
88ab30b 
Signed-off-by: karan k <karan.k@oracle.com>
@karan56625 karan56625 force-pushed the karak/support_file_system_as_storage branch from 95a000b to 88ab30b Compare July 10, 2023 11:38
@benjaminhuo benjaminhuo merged commit 59e9bad into fluent:master Jul 10, 2023
6 checks passed
@benjaminhuo
Copy link
Member

@karan56625 Thank you!

karan56625 pushed a commit to verrazzano/fluent-operator that referenced this pull request Jul 11, 2023
@benjaminhuo @karan56625
Merge pull request fluent#825 from verrazzano/karak/support_file_syst…
2e7b8ca 
…em_as_storage

Support file system as storage layer in service section of fluenbit
karan56625 added a commit to verrazzano/fluent-operator that referenced this pull request Jul 11, 2023
@karan56625 @benjaminhuo
Merge pull request fluent#825 from verrazzano/karak/support_file_syst…
2fe76f7 
…em_as_storage (#14)

Support file system as storage layer in service section of fluenbit

Co-authored-by: Benjamin Huo <benjamin@kubesphere.io>
@benjaminhuo
Copy link
Member

@karan56625 Have add you as the fluent operator maintainer, thanks for your contribution! @wanjunlei @wenchajun @patrick-stephens

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adiforluls adiforluls adiforluls left review comments

@benjaminhuo benjaminhuo benjaminhuo approved these changes

@wenchajun wenchajun wenchajun left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support File system as storage for Input
4 participants
@karan56625 @benjaminhuo @wenchajun @adiforluls