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

[fluent-bit]Use the 'tpl' function for .Values.podAnnotations #437

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

sangheee
Copy link

@sangheee sangheee commented Nov 9, 2023

I want to deploy a custom config with additional template variables, like the fluent-bit.my-config example below.
When this config changes, I want to do a deploy rollout that restarts pods.
Currently, podAnnotations is having difficulty here because they directly use the .Values.podAnnotations variable object.
So, I would like to use the tpl function to evaluate .Values.podAnnotations variable as templates.
Even with this change, backward compatibility is provided.

# _my_config.tpl
{{- define "fluent-bit.my-config" -}}
...
{{- end -}}
# my-values.yaml
fluent-bit:
  podAnnotations:
    checksum/my-config: '{{ include "fluent-bit.my-config" . | sha256sum }}'
    foo: bar

Signed-off-by: sangheee <afk.sh94@gmail.com>
@stevehipwell
Copy link
Collaborator

@sangheee could you explain why neither the hot reload or the config checksum annotation are working for you?

@sangheee
Copy link
Author

sangheee commented Nov 10, 2023

Actually, We create and use a new chart that has a dependency on a fluent-bit chart.
Our chart is for using our complex fluent-bit configuration and deploying other components together when fluent-bit components are deployed.

# my `Chart.yaml`
name: my-chart
dependencies:
  - name: fluent-bit
    version: 0.38.0
    repository: https://fluent.github.io/helm-charts
.my-chart
├── conf
│   ├── filter.conf
│   ├── input.conf
│   └── output.conf
├── templates
│   ├── config.yaml
│   ├── another1.yaml
│   └── another2.yaml
├── Chart.yaml
└── values.yaml

We create a config and set .Values.fluent-bit.existingConfigMap to use it.
So, when this config changes, the checksum must change for rollout to occur, but rollout does not occur with the current structure.
That is why I changed it to add a checksum using the tpl function.

@stevehipwell
Copy link
Collaborator

@sangheee would hot reload not be a better fit here?

@sangheee
Copy link
Author

sangheee commented Nov 13, 2023

@stevehipwell

hot reload is effective when the fluent-bit config is entered as value.
I want to update the annotation and reload pods when the custom config file or something else changes.

      {{- if not .Values.hotReload.enabled }}
        checksum/config: {{ include (print $.Template.BasePath "/configmap.yaml") . | sha256sum }}

What you mean is to modify the code related to hot reload?

@stevehipwell
Copy link
Collaborator

@sangheee hot reload works with the Config maps attached to FlunetBit, it doesn't matter how they were generated.

@sangheee
Copy link
Author

sangheee commented Nov 28, 2023

@stevehipwell Sorry. I was busy with work so I was late in checking.
I also thought about hot reload, but using hot reload causes pods to restart unexpectedly and simultaneously, which we don't want. We want the pods to be restarted sequentially. That's why we can't use hot reload.

@stevehipwell
Copy link
Collaborator

@sangheee hot reload doesn't cause the pod to restart at all, it reloads the Fluent Bit configuration while Fluent Bit is still running.

@JaredTan95
Copy link
Contributor

@sangheee My usage case is the same as yours, I also encountered your problem, and tried the same method your PR try to solved, but in the end I choose the hot-reload method.

@sangheee
Copy link
Author

sangheee commented Apr 29, 2024

Because of the memory leak issue due to multiline, we can not update the version of fluent-bit that supports hot-reload.

@stevehipwell
Copy link
Collaborator

@sangheee could you link to the FB issue regarding the multiline memory leak?

@sangheee
Copy link
Author

@stevehipwell fluent/fluent-bit#4940 This is the issue. Could you give me some advice?

@stevehipwell
Copy link
Collaborator

@sangheee the PR linked in that issue has been merged, so have you checked if your issue is still present in the latest FB release?

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.

3 participants