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

feat(fluent-bit): Added hot-reload support #384

Merged
merged 1 commit into from
Jun 21, 2023

Conversation

stevehipwell
Copy link
Collaborator

No description provided.

@stevehipwell
Copy link
Collaborator Author

This PR is waiting for some external testing and I'll mark it as ready once this has happened.

@stevehipwell
Copy link
Collaborator Author

@edsiper @naseemkullah do you want to take a look at the changes here?

The structural changes are required to support hot-reload as user-overridable files should be in a leaf directory to enable direct Kubernetes ConfigMap or Secret mounting. I'm not sure what the side effects of these changes are but when all paths in the config are absolute I've tested that everything works as expected.

@edsiper
Copy link
Member

edsiper commented Jun 16, 2023

I think @patrick-stephens would be times better than me to review these changes

@stevehipwell
Copy link
Collaborator Author

@edsiper there are some fundamental questions about how Fluent Bit config and workdir works that I asked @patrick-stephens and he passed on to @leonardo-albertovich (I think I got the GH user correct).

Signed-off-by: Steve Hipwell <steve.hipwell@gmail.com>
Copy link
Contributor

@patrick-stephens patrick-stephens left a comment

Choose a reason for hiding this comment

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

Looks good to me, some good improvements here as well as the hot reload support.

I would add something to the docs to explain how hot reload is being done via a watcher sidecar. I think we should look at the requirements on port/webserver usage vs just signals too to confirm.

charts/fluent-bit/templates/_pod.tpl Show resolved Hide resolved
charts/fluent-bit/templates/_pod.tpl Show resolved Hide resolved
charts/fluent-bit/templates/daemonset.yaml Show resolved Hide resolved
charts/fluent-bit/values.yaml Show resolved Hide resolved
charts/fluent-bit/values.yaml Show resolved Hide resolved
charts/fluent-bit/values.yaml Show resolved Hide resolved
charts/fluent-bit/values.yaml Show resolved Hide resolved
charts/fluent-bit/values.yaml Show resolved Hide resolved
charts/fluent-bit/values.yaml Show resolved Hide resolved
charts/fluent-bit/Chart.yaml Show resolved Hide resolved
@stevehipwell
Copy link
Collaborator Author

@patrick-stephens this change came from the two Fluent Bit prototype charts I've been working on (see below) and has been extensively tested (as part of the aggregator chart).

@patrick-stephens
Copy link
Contributor

@stevehipwell I'm happy with this change to merge whenever you are.

@stevehipwell stevehipwell marked this pull request as ready for review June 21, 2023 12:41
@stevehipwell stevehipwell merged commit 6b04c1c into fluent:main Jun 21, 2023
2 checks passed
@stevehipwell stevehipwell deleted the flb-add-hot-reload branch June 21, 2023 12:42
@kyontan
Copy link

kyontan commented Jun 22, 2023

Hi @stevehipwell

I found that the .Values.volumeMounts got ignored after the release of this.
Since I had overridden config volume with the value, now we have no way to override it.
How can we fix the issue?

6b04c1c#diff-d501e88794e6225fbd785cd688bd23d39ffa17e1cafa6f9a704a5b95a73a10d7L88-L89

@stevehipwell
Copy link
Collaborator Author

@kyontan volumeMounts was removed as we now mount the ConfigMap directly. You can use extraVolumes & extraVolumeMounts to mount additional volumes.

Matiasmct pushed a commit to giffgaff/fluent-helm that referenced this pull request Oct 24, 2023
Signed-off-by: Steve Hipwell <steve.hipwell@gmail.com>
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.

None yet

4 participants