Charts: [stable/fluent-bit, stable/fluentd] Move source to a new charts repo

Created on 4 Mar 2020  路  22Comments  路  Source: helm/charts

Hello,

@scottrigby and I are in the process of preparing these charts new home.

Are you all on board and would like to continue maintaining the charts? Please let us know :)

fluent-bit:
@kfox1111
@hectorj2f
@Towmeykaw

fluentd:
@rendhalver
@miouge1
@hectorj2f

lifecyclfrozen

Most helpful comment

Can we try to conclude on the path forward for fluentd?

I think the bitnami chart for fluentd can do mostly the same as the existing stable one does, but it could maybe allow for a "wildcardish" configuration like here: https://github.com/helm/charts/blob/master/stable/fluentd/templates/configmap.yaml#L11.

We moved from the stable to bitnami one at work, and when I look in the flux repo, we apply the configmap outside of helm, using the option to point to an existing cm:
https://github.com/bitnami/charts/blob/master/bitnami/fluentd/values.yaml#L81

The experience when moving would be on par with such a feature I think. WDYT @carrodher ?

All 22 comments

@naseemkullah sounds good.

One of the nice things that the helm/charts repo provides is the CI (helm lint, DCO checker, version bump checker, E2E tests etc...). Do you have plans for some automated CI in the new repo?

@naseemkullah sounds good.

Do you have plans for some automated CI in the new repo?

Indeed we do! We will be using the latest and greatest in github actions for our CI. See https://github.com/helm/charts-repo-actions-demo/ for an example of what we'll be doing.

@naseemkullah 馃憤

@rendhalver @kfox1111 @hectorj2f Please advise as to wether or not you are on board with this! If so you will be retained as maintainers. :) Cheers.

Hey @Miouge1 it seems like the fluentd chart doesnt use an official fluentd image, im not sure we could move it as is, if there were a chart that basically packaged https://github.com/fluent/fluentd-kubernetes-daemonset that could probably work.

Hello @Towmeykaw @kfox1111 @hectorj2f ,

The config values + templating is really unwieldy, if we could achieve something simpler and more flexible (without having to bubble up all possible config options) but in a breaking manner, i think this would be good to do during the moving of the chart. thoughts? Or should we address this like any breaking PR either here or post-move?

I'm thinking something along the lines of:

values file:

config:
  ## Ref: https://docs.fluentbit.io/manual/service
  service:
    Flush: 1
    Daemon: Off
    Log_Level:  info
    Parsers_File: parsers.conf
    Parsers_File: parsers_custom.conf
    HTTP_Server:  On
    HTTP_Listen:  0.0.0.0
    HTTP_Port:    2020

  ## Ref: https://docs.fluentbit.io/manual/input
  inputs:
    - Name: tail
      Path: /var/log/containers/*.log
      Parser: docker
      Tag: kube.*
    - Name: systemd
      Tag: host.*
      Systemd_Filter: _SYSTEMD_UNIT=kubelet.service

  ## Ref: https://docs.fluentbit.io/manual/filter
  filters:
    - Name: kubernetes
      Match: kube.*
      Merge_Log: On
      K8S-Logging.Parser:  On
      K8S-Logging.Exclude: On

  ## Ref: https://docs.fluentbit.io/manual/output
  outputs:
    - Name: es
      Match: "*"
      Host: elasticsearch-master.logging.svc
      Logstash_Format: On

  ## Ref: https://docs.fluentbit.io/manual/parser
  customParsers:
    - Name: docker_no_time
      Format: json
      Time_Keep: Off
      Time_Key: time
      Time_Format: "%Y-%m-%dT%H:%M:%S.%L"

confgmap:

data:
  custom-parsers.conf: |
    {{- range $parser := .Values.config.customParsers }}
    [PARSER]
      {{- range $key, $value := index $parser }}
        {{ $key }} {{ $value }}
      {{- end }}
    {{- end }}

  fluent-bit.conf: |
    [SERVICE]
      {{- range $key, $value := index .Values.config.service }}
        {{ $key }} {{ $value }}
      {{- end }}

    {{- range $input := .Values.config.inputs }}

    [INPUT]
      {{- range $key, $value := index $input }}
        {{ $key }} {{ $value }}
      {{- end }}
    {{- end }}

    {{- range $filter := .Values.config.filters }}

    [FILTER]
      {{- range $key, $value := index $filter }}
        {{ $key }} {{ $value }}
      {{- end }}
    {{- end }}

    {{- range $output := .Values.config.outputs }}

    [OUTPUT]
      {{- range $key, $value := index $output }}
        {{ $key }} {{ $value }}
      {{- end }}
    {{- end }}

It will make the chart way cleaner, mind you if people want to mount secrets and such, they will have to provide that as extraSecrets/extraVolumes and what not. That's to say no more logic based on if backend.foo.bar ....thoughts @kfox1111 @hectorj2f @Towmeykaw ?

I've went ahead and created a fluent-bit chart from scratch https://github.com/fluent/helm-charts/tree/master/charts/fluent-bit based off learnings with the chart in this repo. The config options were getting too unwieldy, the new chart imho has a better approach as per my above comment.

@Towmeykaw @kfox1111 @hectorj2f Please let me know if you would like to help maintain this new one 馃檹

@naseemkullah looks a lot cleaner, I will have a closer look at it but it looks nice. I would love to help maintaining the chart.

Great to hear @Towmeykaw , please open a PR to add yourself as co-maintainer :)

Also I just merged https://github.com/fluent/helm-charts/commit/65860a07fef93e939c818f4eaf11cfd4c7d579a4 which allows for maximum flexibility for volumes (as this chart has) and env vars.

That should be all that's needed to accomodate all plugins! Hopefully we can keep the chart nice and clean :D

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Any further update will cause the issue/pull request to no longer be considered stale. Thank you for your contributions.

This issue is being automatically closed due to inactivity.

@naseemkullah and @scottrigby is https://fluent.github.io/helm-charts fluentd chart has the capability to use config maps. Right now I don't see a way to send fluent.conf at runtime through config map(values.yaml) like we do it in https://kubernetes-charts.storage.googleapis.com/ fluentd-elasticsearch chart. Are we planning to add that capability soon?

@naseemkullah I think https://github.com/fluent/helm-charts was a good move - thanks for all your wok there! End users who are able to update to a cleaner experience can take advantage of the fluent/fluent-bit chart now (it's already being deployed by users for the past few months). If I understand correctly, the fluent/fluentd chart still needs work. This is a great way for the community to get involved there, for those who are able to contribute 馃槃 It's in the fluent community's hands now.

For end users who are not yet able to update to the new experience before 13 Nov (when the incubator and stable repos will be archived and their chart release history garbage collected), where should we point users to?

  • Kiwigrid has adopted stable/fluentd-elasticsearch at https://github.com/kiwigrid/helm-charts/tree/master/charts/fluentd-elasticsearch. Perhaps they would want to move over these other two charts as well?
  • Bitnami has a fluentd chart https://github.com/bitnami/charts/tree/master/bitnami/fluentd. @carrodher has made clear they are very open to collaboration with stable chart maintainers on Bitnami charts as a migration path from the stable ones. Is that chart similar enough to stable/fluentd to be worth collaborating on to ensure it can be the preferred migration path? @rendhalver @Miouge1 @hectorj2f this is really a question for you. If so, @carrodher would Bitnami be open to also adopting the stable/fluent-bit chart as well?

Either way, I think end users will have the best outcome if a conversation/decision/collaboration gets underway sooner than later. What do you all think?

From the Bitnami POV, there are only three requirements to add a new chart to the catalog:

As the bitnami/fluentd chart is already present in our catalog, those requirements are already fulfilled and the existing chart is fully integrated into the test & release pipeline. As the stable/fluentd chart needs to be deprecated sooner rather than later if you want to select the bitnami chart as the continuation of the stable one we can work on:

  • Creating a migration path for users upgrading from stable/fluentd to bitnami/fluentd
  • Adding the features present in the stable chart that are not present in the bitnami one

Can we try to conclude on the path forward for fluentd?

I think the bitnami chart for fluentd can do mostly the same as the existing stable one does, but it could maybe allow for a "wildcardish" configuration like here: https://github.com/helm/charts/blob/master/stable/fluentd/templates/configmap.yaml#L11.

We moved from the stable to bitnami one at work, and when I look in the flux repo, we apply the configmap outside of helm, using the option to point to an existing cm:
https://github.com/bitnami/charts/blob/master/bitnami/fluentd/values.yaml#L81

The experience when moving would be on par with such a feature I think. WDYT @carrodher ?

Hi @davidkarlsen, we are happy to collaborate in a smooth transition from stable/fluentd to bitnami/fluentd, we'll compare both charts adding any missing feature, paying special attention to the cofigMap configuration.

Do you think there is something else we need to take into account in the bitnami/fluentd chart? Something else we need to do apart from trying to implement any missing feature?

I can't see anything else missing than the configurability.

I've been checking both charts and these are the main differences that I could spot:

  • The bitnami/fluentd chart supports a forwarder/aggregator architecture as described here, while the stable/fluentd chart just deploys either a Deployment or a StatefulSet. This simpler architecture can be achieved in bitnami/fluentd by deploying the chart using only the aggregator.

  • The stable/fluentd chart allows the installation of plugins at container initialization. Bitnami doesn't support this and it would be nice to have it supported.

  • The stable/fluentd chart can autoscale itself using a HPA. This could be nice to have in an aggregator-only architecture, but in a forwarder/aggregator architecture it won't work as the forwarders won't be aware of new pods.

  • In the stable/fluentd chart you can configure Fluentd through the values.yaml, while in the Bitnami chart you have to specify an external config map to configure it. Supporting this in Bitnami would be very nice and powerful.

  • stable/fluentd has an Ingress and Bitnami doesn't. This may be a nice feature to have, but I'm not sure if it should be added to the forwarders, the aggregators, or both?

From this list, I'd say the most important items are the "wildcardish" configuration (it will ease the configuration and restart pods on configuration changes as it would be managed by Helm) and the plugin installation at container initialization (although this may require some changes to the container or to the security context of the pods, specially the aggregator pods as they don't run as root by default).


I think that a nice transition between both charts is already possible as shown by @davidkarlsen, but there are a couple of areas (highlighted above) that can be improved. What are your thoughts on this @scottrigby @carrodher @davidkarlsen @Miouge1 ?

I've created a PR to start implementing these missing features. Feel free to add your thoughts there.

@alemorcuq IMHO the plugin installation is not so favourable as it is more fragile compared to just building a ready made image containing the plugins required. But of course - it can be implemented and people can just choose not to use it.

+1 to HPA

+1 to the configuration - which is what I requested too.

I don't care too much about ingress - but it can be optional too.

@alemorcuq - awesome - let me know if you need any help.

Hi all, I would like to let you know that the above PR (https://github.com/bitnami/charts/pull/4215) has been merged in the bitnami repository, at this moment the above-mentioned features are already implemented in the bitnami/fluentd chart (starting on 3.4.0)

Was this page helpful?
0 / 5 - 0 ratings